-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add db paginate #689
base: master
Are you sure you want to change the base?
Add db paginate #689
Conversation
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
amphora
storage modules. - The best place to set / validate
MAX_PAGE_SIZE
andDEFAULT_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 <[email protected]>
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!
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/instances
db.createReadStream()
which has been modified in the storage module, for list requestsprev
andsize
query params to list requestsprev
matches up with where it's being requested, ie. a request tonymag.com/_components/ad/instances
should be for something likenymag.com/_components/ad/instances/aaa
, and dropsprev
if it's invalid, errors if it's notsize
is a number and within the configured maximum, errors if it's not a numberRequires clay/amphora-storage-postgres#64 for pagination.