feat(spanner): add Client Context support to options#1499
feat(spanner): add Client Context support to options#1499aseering wants to merge 10 commits intogoogleapis:mainfrom
Conversation
This change adds support for ClientContext in Options and ensures it is propagated to ExecuteSql, Read, Commit, and BeginTransaction requests. It aligns with go/spanner-client-scoped-session-state design. ClientContext allows passing opaque, RPC-scoped side-channel information (like application-level user context) to Spanner. This implementation supports setting ClientContext at the Client, Database, and Request levels, with request-level options taking precedence. Key changes: - Added ClientContext to types/spanner.py and exposed it. - Updated Client.__init__ to accept a default client_context. - Added helpers for merging ClientContext with correct precedence. - Updated Snapshot, Transaction, Batch, and Database wrappers to propagate the context. - Added comprehensive unit tests in tests/unit/test_client_context.py.
- Update Session.transaction to accept client_context. - Update unit tests to support client_context propagation. - Update mock objects in tests to match the expected attribute hierarchy. - Clean up nested imports in test files.
Implement a double-checked locking pattern in Transaction and _SnapshotBase methods. When multiple threads attempt to use a lazy transaction simultaneously, they race to acquire the lock. Previously, losing threads would acquire the lock and blindly send another 'begin transaction' request, ignoring that the winner had already initialized the transaction ID. This change ensures that threads re-check self._transaction_id after acquiring the lock. If the ID is present, they skip the initialization request and use the established ID.
- Fix critical security vulnerability in where in-place modification of the base object could lead to context leakage across requests. - Replace with throughout , , , and for better robustness and subclass support. - Simplify construction logic in for better readability. Based on suggestions from Gemini Code Assist.
product-auto-label
bot
added
the
api: spanner
Summary of ChangesHello @aseering, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively adds support for ClientContext across the Spanner client library, allowing for more granular control over request options. The changes are consistently applied through various layers of the API, from the client down to individual transactions and snapshots. The addition of a comprehensive test suite for the new functionality is also a great inclusion. I have one suggestion to improve maintainability by refactoring a duplicated block of code into a shared helper function.
google/cloud/spanner_v1/batch.py
Outdated
| if client_context is not None: | ||
| if isinstance(client_context, dict): | ||
| client_context = ClientContext(client_context) | ||
| elif not isinstance(client_context, ClientContext): | ||
| raise TypeError("client_context must be a ClientContext or a dict") | ||
| self._client_context = client_context |
There was a problem hiding this comment.
This block of code for validating and converting client_context is duplicated in several places (e.g., _BatchBase, MutationGroups, Client, _SnapshotBase). To improve maintainability and reduce code duplication, consider extracting this logic into a helper function in google/cloud/spanner_v1/_helpers.py.
3 participants
Re-opening #1495 due to permissions issues.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕