- Notifications
You must be signed in to change notification settings - Fork321
fix: apply timeout to all resumable upload requests#1070
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
tswast commentedNov 18, 2021
Test failure: I think we'll have to bump the minimum version of the resumable media library. |
tseaver commentedNov 18, 2021
@tswast The feature was added (by@plamut) in June 2020, and released with version 0.6.0 of The test failure comes from thebroken stub version in the testcase. |
| may be repeated several times using the same timeout each time. | ||
| Can also be passed as a tuple (connect_timeout, read_timeout). | ||
| See :meth:`requests.Session.request` documentation for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm curious, does the same apply to our other API requests? I know we userequests library for those too, but viagoogle-cloud-core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
From what I can see, it does. Thetimeout argument gets passed down togoogle.cloud._http.JSONConnection, whichaccepts a two-tuple.
Ithink we did not advertise this second option in BigQuery and just went withOptional[Union[int, float]] to avoid leaking transport implementation details, but I'm not 100%.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
google-cloud-coredefinitely exposes bothfloat andtuple fortimeout.
Note that its current docs link (undercloud.google.com) doesn't show those details.
Uh oh!
There was an error while loading.Please reload this page.
* fix: apply timeout to all resumable upload requests* Fix stub in test case* Improve timeout type and other type annotations* Annnotate return type of _do_resumable_upload()
Fixes#1067.
PR checklist: