Repository navigation
Conversation
…s none RESTClientObject.request sent `timeout=_request_timeout or 5 * 60` on every request. A per-request timeout replaces the client's, so a client set as pool_manager with its own timeout still waited up to 5 minutes. A call without _request_timeout now uses the client's timeout, and the client from _create_pool_manager keeps the 5 minute default.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
Member
|
thanks for the PR cc @cbornet (2017/09) @tomplus (2018/10) @krjakbrjak (2023/02) @fa0311 (2023/10) |
This branch has not been deployed
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
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.
The httpx and httpx2
rest.pysendstimeout=_request_timeout or 5 * 60on every request. In HTTPX a per-request timeout replaces the client's, so a client set aspool_managerwith its own timeout still waits up to 5 minutes unless every call passes_request_timeout.Configurationhas no timeout setting, so there is no other way to change it.This change moves the 5 minute default from the request to the client:
_request_timeoutuses{{httpxModule}}.USE_CLIENT_DEFAULT, so the client's own timeout applies._create_pool_managerbuilds itsAsyncClientwithtimeout=5 * 60, so clients the generator creates keep the current 300 s default.The urllib3
rest.pyalready behaves this way: without_request_timeoutit leaves the timeout unset and uses the pool manager's. The aiohttp template has the same5 * 60line but a different timeout API, so it is left out of this PR.Validation: regenerated the four httpx samples (
python-httpx,python-httpx-sync,python-httpx2,python-httpx2-sync); onlyrest.pychanged. Added two tests to the httpx2 samples'test_httpx2_transport.py: a supplied client's timeout applies when the call sets none, and the generated client defaults to 300 s. Both fail on master and pass with this change. The sample suites pass excepttest_pet_api.py, which needs a local Petstore server.PR checklist
./bin/generate-samples.sh bin/configs/python-httpx*.yaml, the only configs using this template.Python technical committee: @wing328
Summary by cubic
Fixes the httpx Python templates so calls without
_request_timeouthonor the pool manager's own timeout instead of always forcing a 5-minute default. The 5-minute default is now set on the generatedAsyncClientin_create_pool_manager, so generated clients keep the current 300-second timeout._request_timeoutnow sendhttpx.USE_CLIENT_DEFAULTso the client's timeout applies.rest.pychanged. The aiohttp template is unchanged.Written for commit e322f84. Summary will update on new commits.