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

Respect argument-level timeout #476

Open
CA-Lee opened this issue Jun 5, 2022 · 1 comment
Open

Respect argument-level timeout #476

CA-Lee opened this issue Jun 5, 2022 · 1 comment
Labels

Comments

@CA-Lee
Copy link

CA-Lee commented Jun 5, 2022

session_uri = await self._initiate_upload(url, object_name, params,

As refered above, this call to _initiate_upload() in _upload_resumable() does not pass timeout argument which may be set on calling upload() (in same class, Storage). Result in all function called in _initiate_upload() respect to its default timeout, which is DEFAULT_TIMEOUT in _initiate_upload().

I'm not sure if it is intended or not; if so, I would recommend to add some explanation; if not, I would glad to open a PR for this.

Thanks for you guys creating this useful stuff.

@TheKevJames
Copy link
Member

Thanks for the report! Yeah, this was originally intended (though in retrospect might be a bad idea!) as I felt as a user setting a "timeout value for an upload" was most reasonably likely to refer to a timeout for the upload itself rather than for a specific part of the handshake. That said, I'd certainly be open to changing this if folks think it's unclear!

Some options to consider:

  • leave as is, document this better
  • use the current timeout value for both the initiate and the "do" steps
  • add a new initiate_timeout param to plumb into the initiate step

I think personally I would lean to the last of those options, as I'd rather keep things explicit. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants