Skip to content
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

Set default Search.max_results to 100 #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lukasschwab
Copy link
Owner

Description

Defaulting to None yields a bad default user experience: if you provide too open-ended a query (e.g. "testing" or "quantum," examples I use in the docs) and try dumping into list(...) rather than using a generator, max_results=None with the default page_size will appear to hang.

In practice, one should only set max_results=None in a loop that processes results incrementally or in a long-running process or with a query where the result set is known to be small.

Breaking changes

List any changes that break the API usage supported on master.

Changes default Search behavior.

This is arguably a breaking change, but I think it may be appropriate to bundle it into a minor-version release: the existing default behavior is pathological besides on small result sets.

Relevant issues

List GitHub issues relevant to this change.

Checklist

  • (If appropriate) README.md example usage has been updated.

@lukasschwab lukasschwab marked this pull request as ready for review October 19, 2023 01:41
Base automatically changed from docs/rm-superfluous-module-nesting to master October 19, 2023 01:43
@lukasschwab lukasschwab changed the title Set sefault Search.max_results to 100 Set default Search.max_results to 100 Oct 23, 2023
Defaulting to None yields a bad default user experience: if you provide
too open-ended a query (e.g. "testing" or "quantum," examples I use in
the docs) and try dumping into `list(...)` rather than using a
generator, `max_results=None` with the default `page_size` will appear
to hang.

In practice, one should *only* set `max_results=None` in a loop that
processes results incrementally *or* in a long-running process *or* with
a query where the result set is known to be small.

This is arguably a breaking change, but I think it may be appropriate to
bundle it into a minor-version release.
@lukasschwab lukasschwab force-pushed the defaults/limit-default-max_results branch from 387ac55 to b3c68fa Compare January 14, 2024 01:08
Copy link

@antonkesy antonkesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants