Conversation
mattoberle
suggested changes
Dec 7, 2020
Contributor
mattoberle
left a comment
mattoberle
left a comment
There was a problem hiding this comment.
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).
mattoberle
reviewed
Dec 9, 2020
Co-authored-by: Matt Oberle <matt.r.oberle@gmail.com>
mattoberle
approved these changes
Dec 10, 2020
Contributor
mattoberle
left a comment
mattoberle
left a comment
There was a problem hiding this comment.
👍
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!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.