Repository navigation
feat(datastore): add x-goog-is-transaction routing header - #18848
nikhilchowdary wants to merge 1 commit into
Conversation
This commit attaches the `x-goog-is-transaction` routing metadata header to all RPCs in `google-cloud-datastore` (both gRPC and HTTP transports): - Set to `true` for transactional RPCs (`begin_transaction`, `rollback`, and transactional `commit`, `lookup`, `run_query`, `run_aggregation_query`). - Set to `false` for non-transactional RPCs (`allocate_ids`, `reserve_ids`, and non-transactional `commit`, `lookup`, `run_query`, `run_aggregation_query`).
There was a problem hiding this comment.
Code Review
This pull request introduces transaction metadata tracking across Google Cloud Datastore client requests by adding a 'metadata' parameter to propagate the 'x-goog-is-transaction' header. This header is set dynamically depending on whether the request is transactional or non-transactional. Feedback on these changes includes a recommendation to use 'headers.update(metadata)' instead of a 'for' loop to avoid potential 'ValueError' exceptions when 'metadata' is a dictionary, as well as a suggestion to format multi-line function calls in accordance with PEP 8.
| if metadata: | ||
| for k, v in metadata: | ||
| headers[k] = v |
There was a problem hiding this comment.
Using a for k, v in metadata loop to unpack metadata will raise a ValueError if metadata is passed as a dictionary (since iterating over a dict yields only its keys).
Using headers.update(metadata) is more robust and idiomatic, as dict.update() natively supports both dictionaries and iterables of key-value pairs (like tuples of tuples).
if metadata:
headers.update(metadata)| retry=retry, | ||
| timeout=timeout, | ||
| ) | ||
| timeout=timeout, metadata=metadata) |
There was a problem hiding this comment.
Add x-goog-is-transaction routing header in python SDK to differentiate transactional requests and non-transactional requests from each other. Having a routing header for transactions helps in routing grpc requests at GFE.
This commit attaches the
x-goog-is-transactionrouting header to all RPCsSet to true for transactional RPCs
--
beginTransaction--
rollback--
transactional commit--
transactional lookup--
transactional runQuery--
transactional runAggregationQuery.Set to false for non transactional RPCs
--
allocateIds--
non transactional commit--
non transactional lookup--
non transactional runQuery--
non transactional runAggregationQuery.cc: @MarkDuckworth