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

Document thread-safety of library #5685

Closed
BrandonY opened this issue Jul 25, 2018 · 5 comments · Fixed by #5763
Closed

Document thread-safety of library #5685

BrandonY opened this issue Jul 25, 2018 · 5 comments · Fixed by #5763
Assignees
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: docs Improvement to the documentation for an API.

Comments

@BrandonY
Copy link

After reading through the docs, it is unclear to me whether I should expect this code to work:

from google.cloud import storage
import thread

client = storage.Client()
for blob in bucket.list_blobs():
  thread.start_new_thread( blob.download_to_filename, '/tmp/' + blob.name)
@tseaver tseaver added api: storage Issues related to the Cloud Storage API. type: docs Improvement to the documentation for an API. labels Jul 25, 2018
@tseaver
Copy link
Contributor

tseaver commented Jul 25, 2018

google-cloud-storage has been based on requests since #3674, released with google-cloud-storage 1.3.0 just about a year ago.

I'm leaving this open as a docs bug so that we add a note to that effect.

@tseaver tseaver added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Jul 25, 2018
@tseaver tseaver self-assigned this Jul 25, 2018
@BrandonY
Copy link
Author

Sorry, you last me. Does being based on requests imply that google-cloud-storage is thread-safe, and the code example above should work fine?

@tseaver
Copy link
Contributor

tseaver commented Jul 26, 2018

Yes to both questions.

@tseaver
Copy link
Contributor

tseaver commented Aug 7, 2018

@BrandonY Here is the text I am planning to add to the client page:

.. note::

   Becuase the :class:`~google.cloud.storage.client.Client` uses the
   third-party :mod:`requests` library by default, it should be safe to
   share instances across threads.  In multiprocessing scenarious, best
   practice is to create client instances *after* 
   :class:`multiprocessing.Pool` or :class:`multiprocessing.Process` invokes
   :func:`os.fork`.

Does that address your need?

@tseaver
Copy link
Contributor

tseaver commented Aug 7, 2018

@BrandonY Please follow up via #5763, if needed.

tseaver added a commit that referenced this issue Aug 20, 2018
Also note general best practice for multiprocessing use.

Closes #5685.
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. priority: p2 Moderately-important priority. Fix may not be included in next release. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants