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 database page limit #139
Conversation
Codecov Report
@@ Coverage Diff @@
## master #139 +/- ##
==========================================
+ Coverage 86.12% 86.18% +0.06%
==========================================
Files 39 39
Lines 1816 1824 +8
==========================================
+ Hits 1564 1572 +8
Misses 252 252
Continue to review full report at Codecov.
|
a1481ea
to
2cc22ee
Compare
optimade/server/entry_collections.py
Outdated
) | ||
if limit == 0: | ||
limit = self.page_limit | ||
limit = CONFIG.page_limit |
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.
is this necessary?
what if limit < 0?
If people explicitly request 0 entries, I'd say they should get 0 entries
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.
what if limit < 0?
This is handled by the QueryParams, i.e., all query parameter of page_*
are NonNegativeInt
types.
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.
But I guess we could put in another check, although it shouldn't be necessary?
Update handling of `page_limit` in `_parse_params()`.
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.
looks good to me!
Adds the concept a maximum database limit to what the database will return in a single request.
Essentially it sets a maximum limit for the
page_limit
query parameter.This makes the page_limit concept from the config files a default page_limit.
Note: This is based on the work from #137, so this should not be merged before that PR, and should be merged via "Rebase and merge".