-
Notifications
You must be signed in to change notification settings - Fork 4.3k
tests: add async timeout/retry coverage and clarify per-request overrides #2595
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
Open
lucasalencarxisto-stack
wants to merge
1
commit into
openai:main
from
lucasalencarxisto-stack:tests/async-timeouts-retries
Open
tests: add async timeout/retry coverage and clarify per-request overrides #2595
lucasalencarxisto-stack
wants to merge
1
commit into
openai:main
from
lucasalencarxisto-stack:tests/async-timeouts-retries
+443
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@lucasalencarxisto-stack
lucasalencarxisto-stack
requested a review
from a team
as a code owner
August 31, 2025 15:13
Hi team 👋
This PR addresses a test issue I encountered while working with timeouts/retries:
- Fixed
UnboundLocalErrorin async timeout tests by removing inline type hints and ensuringhttpxis imported only at the top level. - Added explicit async coverage to confirm per-request timeouts override client defaults, and that smaller per-request values properly raise
TimeoutException. - Improved retry tests with clearer comments and docstrings, especially around handling
Retry-Afterheaders and server error retries.
The main goal is to make timeout/retry behavior more robust against regressions and easier to understand for future contributors.
Happy to adjust anything if needed 🙏
OscaeGTX
OscaeGTX
approved these changes
Sep 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
Note: This PR overlaps with #2615 / #2616.
If those land first, I’m happy to close this one.
The goal here is simply to improve async timeout/retry coverage and clarify per-request overrides.
Summary
This PR improves the test suite for timeouts and retries by adding clearer
async coverage and detailed assertions for per-request overrides.
Changes
test_timeouts_async.pywith:TimeoutException.float,dict, orhttpx.Timeout.test_timeouts.pyandtest_retries.py.OPENAI_API_KEYwith a dummy value during tests.Why
Retry-Afterheaders are present (sync + async).Notes
UnboundLocalErrorin async handlers by removing inline type hints.