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

Remove QueryTimeout#isTimeoutEnabled method and move check to caller #11954

Merged
merged 4 commits into from
Nov 24, 2022

Conversation

shubhamvishu
Copy link
Contributor

Description

This removes the method QueryTimeout#isTimeoutEnabled and moves the responsibility to ensure timeout is not null to the caller.
Initially I went with the approach to allow QueryTimeout to be null (and removing #isTimeoutEnabled) and allow wrapping TimeLimitingBulkScorer and ExitableDirectoryReader with null timeouts which is equivalent of timeout not enabled as this makes work easy for the caller but giving a thought again it made more sense to only wrap the classes if there is an actual timeout and moving the responsibility to ensure timeout is configured to caller method as mentioned in the issue.

Closes #11914

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

This looks good, can you add a CHANGES entry under version 9.5.0?

@shubhamvishu
Copy link
Contributor Author

Add entry in CHANGES.txt

Thanks for reviewing @jpountz 😀 ...... I have added the entry under 9.5.0.

@jpountz jpountz merged commit b15ace4 into apache:main Nov 24, 2022
jpountz pushed a commit that referenced this pull request Nov 24, 2022
@shubhamvishu shubhamvishu deleted the querytimeout branch November 26, 2022 11:26
dantuzi pushed a commit to SeaseLtd/lucene that referenced this pull request Dec 29, 2022
@rmuir rmuir added this to the 9.5.0 milestone Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove QueryTimeout#isTimeoutEnabled?
3 participants