-
Notifications
You must be signed in to change notification settings - Fork 22
enforce exact max results in search_entities #907
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: master
Are you sure you want to change the base?
enforce exact max results in search_entities #907
Conversation
| while True: | ||
| params.update({'continue': cont_count}) | ||
| max_page_results = min(50, max_results - cont_count) | ||
| params.update({'continue': cont_count, 'limit': max_page_results}) |
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.
More than open to style and logic suggestions.
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.
BTW, it seems we could move the cont_count < max_results criteria into the While condition above (and save 3 lines at the bottom of the function).
Added explicit type annotation to the 'params' dictionary in the search_entities function for improved code clarity and type checking.
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.
Pull Request Overview
This PR enforces strict adherence to the maximum results limit in the search_entities function by calculating page-specific limits instead of using a fixed 50-result page size. This prevents accidentally retrieving more results than requested and protects downstream systems from being overwhelmed with data.
- Modified the pagination logic to respect the exact
max_resultsparameter - Added dynamic calculation of page size based on remaining results needed
- Enhanced type annotations for the params dictionary
| max_page_results = min(50, max_results - cont_count) | ||
| params.update({'continue': cont_count, 'limit': max_page_results}) |
Copilot
AI
Sep 19, 2025
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.
The calculation max_results - cont_count can result in negative or zero values when cont_count >= max_results, which would cause max_page_results to be 0 or negative. This could lead to API errors or infinite loops. Add a check to break the loop when cont_count >= max_results before calculating max_page_results.
Now "max" means max.
This prevents overwhelming downstream systems, especially if the developer loads these into a database without realizing. #askingForAFriend ;)