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(storage): add timeout parameter to public methods #10210

Closed
wants to merge 9 commits into from

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jan 27, 2020

Closes #10199.
Towards #10182.

Preview only for now to show the direction this is going. There are still quite a few potentially blocking code paths that are not covered yet.

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) .

Things still to do/check:

  • Add timeout to the remaining methods that use client._connection.api_request() (read: helpers)
  • Add timeout to any methods that use _connection._make_request() directly, i.e. bypassing the api_request() method.
  • Add timeout to public methods that interact with methods using the timeout (to pass the timeout through).

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 type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: storage Issues related to the Cloud Storage API. labels Jan 27, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 27, 2020
@plamut
Copy link
Contributor Author

plamut commented Feb 7, 2020

Replaced by a PR in the new repo - googleapis/python-storage#44

@plamut plamut closed this Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. 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.

None yet

2 participants