-
Notifications
You must be signed in to change notification settings - Fork 133
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
Paginate data list responses after a configurable threshold #2010
Conversation
0afc725
to
1220ff8
Compare
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
pagination_keys = [self.paginator.page_query_param, | ||
self.paginator.page_size_query_param] | ||
query_param_keys = self.request.query_params | ||
should_paginate = any([k in query_param_keys for k in pagination_keys]) | ||
|
||
if not should_paginate and not is_public_request: |
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.
Does this mean we will not paginate public forms that exceed the threshold?
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.
At the moment yes; didn't think of this while implementing the feature... Any thoughts/reservations with always paginating public requests? Seems a bit painful to calculate the entire count(getting all public XForms and summing the num_of_submissions
) of public data that's currently available...
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.
This is for a single form request at a time, not all public forms at the same. Also, we could go with num_of_submissions
that is not calculated on the fly, it is a safe estimate in most situations.
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.
I withdraw my statement, I see that is_public_request represents access to the /api/v1/data/public
and not to one single form data.
Changes / Features implemented
Steps taken to verify this change does what is intended
Side effects of implementing this change
Closes #2008