-
-
Notifications
You must be signed in to change notification settings - Fork 205
fix(data): update /api/v2/data/ limits DEV-1278
#6472
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
base: main
Are you sure you want to change the base?
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.
You need to change the behaviour in the data viewset.
If no limit is specified, it will always take the max limit while we want to return the default limit. The max limit should only cap a specified limit.
Moreover: Do not forgot to update the documentation. It still says 30000 ;-)
…78-data-endpoint-pagination
…ctly and update documentation
kpi/views/v2/data.py
Outdated
| ) | ||
| else: | ||
| # If no limit is specified, use the default limit (100) | ||
| filters['limit'] = 100 |
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 would use self.pagination_class.default_limit instead. To avoid redundancy.
We could even use self.pagination_class.max_limit instead of settings.SUBMISSION_LIST_LIMIT. What do you think?
noliveleger
left a comment
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.
LGTM
🗒️ Checklist
#Support Docs Updates, if any<type>(<scope>)<!>: <title> DEV-1234Front endand/orBack endorworkflow📣 Summary
Change maximum number of results that
/api/v2/data/can return to 100 by default and 1000 maximum.👀 Preview steps
/api/v2/assets/{uid_asset}/data/