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

feat: distinguish transport and execution time timeouts #424

Merged
merged 4 commits into from
Jan 22, 2020

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jan 17, 2020

Closes #423.
Towards https://github.com/googleapis/google-cloud-python/issues/10137

This PR makes a distinction between the Requests transport timeout, i.e. the timeout parameter, and the total allowed execution time for the request() method, i.e. the max_allowed_time argument. These two are not mixed together anymore.

In other words, if timeout is passed in (set to, say, 60 seconds) and an underlying request takes longer, but the times between the bytes received by the socket are all under 60 seconds, the TimeoutGuard should not kick in and cause a timeout error (the cause of Storage #10137).

TimeoutGuard is only affected by max_allowed_time, which is None by default, making the guard a no-op.

In Requests transport treat ``timeout`` stricly as a transport
timeout, while the total allowed method execution time can be set
with an optional ``max_allowed_time`` argument.
@plamut plamut added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Jan 17, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 17, 2020
tests/transport/test_requests.py Outdated Show resolved Hide resolved
google/auth/transport/requests.py Show resolved Hide resolved
google/auth/transport/requests.py Show resolved Hide resolved
@busunkim96 busunkim96 merged commit 52a733d into googleapis:master Jan 22, 2020
@plamut plamut deleted the iss-423-rework-timeout branch January 22, 2020 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make a clear distinction between the transport timeout and the total method time timeout
3 participants