Paginate Site Evaluations endpoint #109
Merged
Conversation
This file contains 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
mvandenburgh
commented
Nov 2, 2022
mcovalt
approved these changes
Nov 7, 2022
There was a problem hiding this comment.
This looks good to me! @mvandenburgh be sure to review my changes in 0875f7d. Also note the subsequent suppression of MyPy warning in ead97b5.
The commit 0875f7d changes the behavior slightly so that count, timerange, and bbox cover the whole object list, rather than the paginated subset. Unfortunately, my change brings us to two queries that I hope we can reduce to just one in the future with a Subquery.
|
Changes look good to me, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #89
Unfortunately, DRF's default pagination mechanism doesn't completely work here. DRF's
paginate_querysetmethod returns alistinstead of aQuerySet, and thus is intended to be called after all other queries have been made. Unfortunately, the end result of the queries made in the site evaluations endpoint is not aQuerySetbut adict, due to the use of.aggregate(). So, there doesn't appear to be a way to use DRF's native pagination methods here; instead, I've dropped down a level to Django's nativePaginator, which the DRF one uses under the hood.I went ahead and configured pagination for DRF anyway in
settings.py, and also refer to thePAGE_SIZEsetting in the manual pagination code I wrote so it will be consistent with any DRF-paginated endpoints we have in the future.