Skip to content

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Feb 4, 2020

Closes #6.
Closes #13.

A replacement for the same PR in the old repo.

Notes

  • ACL._ensure_loaded() has a default timeout, but not all internal calls to it can be made that configurable, such as in ACL.__iter__(). I thus left that part to use a default timeout.
  • Blob.download_to_file(), Blob.upload_from_file(), and the methods that rely on these two do not expose timeouts, because the underlying resumable-media logic applies its own default timeout to transport.request(), which would override user timeouts.
    Getting around that would first require adding configurable timeouts to the resumable-media dependency (there is a PR #116 that adds these, but only for uploads and it's not merged yet) .

PR checklist:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@plamut plamut added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Feb 4, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 4, 2020
@plamut plamut changed the title feat(storage): add timeout parameter to public methods feat: add timeout parameter to public methods Feb 4, 2020
@plamut
Copy link
Contributor Author

plamut commented Feb 11, 2020

System tests CI failure seems to be temporary (503 ServiceUnavailable).

@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 11, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 11, 2020
@crwilcox crwilcox merged commit 63abf07 into googleapis:master Feb 11, 2020
@plamut plamut deleted the iss-10199 branch June 15, 2020 14:31
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* Add a default request timeout constant

* Add timeout to ACL public methods

* Add timeout to Blob methods

* Add default timeout to Batch._do_request()

* Add timeout to Bucket methods

* Add timeout to Client methods

* Add timeout to HMACKeyMetadata methods

* Add timeout to BucketNotification methods

* Add timeout to _PropertyMixin helpers
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* Add a default request timeout constant

* Add timeout to ACL public methods

* Add timeout to Blob methods

* Add default timeout to Batch._do_request()

* Add timeout to Bucket methods

* Add timeout to Client methods

* Add timeout to HMACKeyMetadata methods

* Add timeout to BucketNotification methods

* Add timeout to _PropertyMixin helpers
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.

Storage: No timeouts cause indefinite hanging Storage: add timeout parameter to all public methods
4 participants