Generate datastore grpc client outside request handler#846
Generate datastore grpc client outside request handler#846cobookman wants to merge 1 commit intoGoogleCloudPlatform:masterfrom
Conversation
Generating a datastore grpc client for each request is not required and can lead to auth issues. Here is one of the auth issues in question: googleapis/google-cloud-python#3085 Also note the following issue which is causing the client to need resetting every 30 mins: googleapis/google-cloud-python#2896. This will hopefully be fixed soon.
|
Datastore isn't always gRPC, and when it isn't, it certainly isn't threadsafe. I'm actually not sure if gRPC datastore is completely threadsafe. @dhermes @lukesneeringer |
|
I am also not sure if gRPC clients are threadsafe. Who would know? |
|
Not sure on Python implementation. But other languages have threadsafe grpc
clients.
https://groups.google.com/forum/m/#!topic/grpc-io/-jA_JCiugM8
googleapis/google-cloud-ruby#816
I can raise the grpc clients threadsafety in Python internally if need be.
…On Fri, Mar 10, 2017, 10:33 AM Danny Hermes ***@***.***> wrote:
I am also not sure if gRPC clients are threadsafe. Who would know?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#846 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABqI55tTq1Sgc8Mv0SP9DD2IBhjp0F6Sks5rkZeKgaJpZM4MY5K8>
.
|
|
@cobookman the gRPC level stuff is threadsafe, I'm just not sure if google-cloud-python's Client object is. |
|
I know that in other languages creating a grpc client in each request handler can cause authentication errors to spawn. |
|
Anyone here have a contact who can inform us if the python grpc client is threadsafe? |
|
Tagging @nathanielmanistaatgoogle, who might know. |
|
gRPC Python is designed for multithreaded use cases and only a few of its operations aren't thread-safe (usually for performance reasons). What specific objects, classes, methods, and functions are under consideration in this circumstance? |
|
@nathanielmanistaatgoogle making a client connection to datastore in a single thread, then sharing that datastore client across threads (aka webhandlers). |
|
Again, to clarify, I'm not concerned with the grpc-level objects, I'm more concerned with the stuff implemented here which @lukesneeringer and @dhermes should be able to comment on. Finally, even if the gRPC implementation is thread-safe, the fallback http implementation is not. Considering it's possible for users to use the non-thread-safe path, I'd prefer if we kept the sample as-is. |
|
@cobookman: I think it's highly likely that the client connection uses gRPC Python ( The only real client-side thread-safety issue for gRPC Python that I can imagine is if a response-streaming RPC is invoked and then the response iterator returned by that RPC invocation is passed to multiple threads which each try to iterate over it. That's a reasonable enough thing to want to do in theory, but it's rare enough in practice that it made sense for us to decide that we weren't going to incur lock acquisition for every case just to support that case. And it's easy enough to write a serializing decorator object for an iterator. |
|
Given @nathanielmanistaatgoogle's comments. Should we continue merging this change in or do we want to close it due to the small chance of a python client not using gRPC. |
|
I'd still like for @lukesneeringer to chime in on the threadsafety of the datastore client class itself. |
|
Closing due to inactivity, but if this is still relevant please feel free to open a new PR. Fwiw, I think that the Datastore client is now thread-safe. |
* feat: Allow users to explicitly configure universe domain chore: Update gapic-generator-python to v1.14.0 PiperOrigin-RevId: 603108274 Source-Link: googleapis/googleapis@3d83e36 Source-Link: https://github.com/googleapis/googleapis-gen/commit/baf5e9bbb14a768b2b4c9eae9feb78f18f1757fa Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYmFmNWU5YmJiMTRhNzY4YjJiNGM5ZWFlOWZlYjc4ZjE4ZjE3NTdmYSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: Resolve AttributeError 'Credentials' object has no attribute 'universe_domain' fix: Add google-auth as a direct dependency fix: Add staticmethod decorator to methods added in v1.14.0 chore: Update gapic-generator-python to v1.14.1 PiperOrigin-RevId: 603728206 Source-Link: googleapis/googleapis@9063da8 Source-Link: https://github.com/googleapis/googleapis-gen/commit/891c67d0a855b08085eb301dabb14064ef4b2c6d Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiODkxYzY3ZDBhODU1YjA4MDg1ZWIzMDFkYWJiMTQwNjRlZjRiMmM2ZCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(diregapic): s/bazel/bazelisk/ in DIREGAPIC build GitHub action PiperOrigin-RevId: 604714585 Source-Link: googleapis/googleapis@e4dce13 Source-Link: https://github.com/googleapis/googleapis-gen/commit/4036f78305c5c2aab80ff91960b3a3d983ff4b03 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDAzNmY3ODMwNWM1YzJhYWI4MGZmOTE5NjBiM2EzZDk4M2ZmNGIwMyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(deps): Require `google-api-core>=1.34.1` fix: Resolve issue with missing import for certain enums in `**/types/…` PiperOrigin-RevId: 607041732 Source-Link: googleapis/googleapis@b453267 Source-Link: https://github.com/googleapis/googleapis-gen/commit/cd796416f0f54cb22b2c44fb2d486960e693a346 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiY2Q3OTY0MTZmMGY1NGNiMjJiMmM0NGZiMmQ0ODY5NjBlNjkzYTM0NiJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(deps): Exclude google-auth 2.24.0 and 2.25.0 chore: Update gapic-generator-python to v1.14.4 PiperOrigin-RevId: 611561820 Source-Link: googleapis/googleapis@87ef1fe Source-Link: https://github.com/googleapis/googleapis-gen/commit/197316137594aafad94dea31226528fbcc39310c Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMTk3MzE2MTM3NTk0YWFmYWQ5NGRlYTMxMjI2NTI4ZmJjYzM5MzEwYyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat: Add include_recaptcha_script for as a new action in firewall policies PiperOrigin-RevId: 612851792 Source-Link: googleapis/googleapis@49ea2c0 Source-Link: https://github.com/googleapis/googleapis-gen/commit/460fdcbbbe00f35b1c591b1f3ef0c77ebd3ce277 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNDYwZmRjYmJiZTAwZjM1YjFjNTkxYjFmM2VmMGM3N2ViZDNjZTI3NyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(deps): require google-api-core>=1.34.1,>=2.11.0, google-auth >= 2.14.1 * filter warning in generated tests --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Generating a datastore grpc client for each request is not required and can lead to auth issues.
Here is one of the auth issues in question: googleapis/google-cloud-python#3085
Also note the following issue which is causing the client to need resetting every 30 mins: googleapis/google-cloud-python#2896. This will hopefully be fixed soon.