-
Notifications
You must be signed in to change notification settings - Fork 24
Add db paginate #689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add db paginate #689
Conversation
mattoberle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take some time to chat about CLAY_STORAGE_PAGE_SIZE, CLAY_STORAGE_MAX_PAGE, and .paginate.
I think this is really close, but we need to hammer out:
- Compatibility with other
amphorastorage modules. - The best place to set / validate
MAX_PAGE_SIZEandDEFAULT_PAGE_SIZE. - The best naming convention for those two settings.
I know you mentioned moving the settings into Amphora which I think is a decent option (if we could leave it up to the storage method to implement/use the additional previous and size opts).
Co-authored-by: Matt Oberle <matt.r.oberle@gmail.com>
mattoberle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
We talked about one additional test case for CLAY_STORAGE_DEFAULT_PAGE_SIZE=undefined and CLAY_STORAGE_MAX_PAGE_SIZE=int and then this seems good to go to me!
jjpaulino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍕
Sets us up for pagination to list endpoints like
/_components/instancesdb.createReadStream()which has been modified in the storage module, for list requestsprevandsizequery params to list requestsprevmatches up with where it's being requested, ie. a request tonymag.com/_components/ad/instancesshould be for something likenymag.com/_components/ad/instances/aaa, and dropsprevif it's invalid, errors if it's notsizeis a number and within the configured maximum, errors if it's not a numberRequires clay/amphora-storage-postgres#64 for pagination.