Skip to content

Conversation

surbhigarg92
Copy link
Contributor

No description provided.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/python-spanner API. labels Mar 4, 2025
@surbhigarg92 surbhigarg92 force-pushed the snapshot_isolation branch 2 times, most recently from a300f1e to 44e0104 Compare March 4, 2025 17:09
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Mar 6, 2025
@surbhigarg92 surbhigarg92 marked this pull request as ready for review March 6, 2025 05:20
@surbhigarg92 surbhigarg92 requested review from a team as code owners March 6, 2025 05:20
@surbhigarg92 surbhigarg92 force-pushed the snapshot_isolation branch 4 times, most recently from 7f671bb to f6f44b6 Compare March 6, 2025 17:17
@surbhigarg92
Copy link
Contributor Author

@olavloite Can you please do one more round of review. Note: Python does not have a similar method like "mergeFrom" in Java. It would need a custom method to merge protos iteratively.
So for Python I have not used proto merge and simply fetching the value directly.

@parthea parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
@surbhigarg92 surbhigarg92 force-pushed the snapshot_isolation branch 4 times, most recently from 497bf85 to 7538b59 Compare March 11, 2025 09:24
@@ -165,6 +167,10 @@ class Client(ClientWithProject):
or you can use the environment variable `SPANNER_ENABLE_END_TO_END_TRACING=<boolean>`
to control it.

:type default_transaction_options: :class:`~google.cloud.spanner_v1.DefaultTransactionOptions`
or :class:`dict`
:param default_transaction_options: (Optional) Default options to use for all transactions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we clearly call out that this is only for r/w transactions?

Suggested change
:param default_transaction_options: (Optional) Default options to use for all transactions.
:param default_transaction_options: (Optional) Default options to use for all read/write transactions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep it flexible for all types of transaction options. For example, if we add exclude_txn_from_change_streams to the default transaction options, it should apply to both RW and PDML. To handle this, we can introduce another private property _defaultPDMTransactionOptions within the DefaultTransactionOptions class, where exclude_txn_from_change_streams will be set.

@surbhigarg92 surbhigarg92 enabled auto-merge (squash) March 12, 2025 06:40
@surbhigarg92 surbhigarg92 merged commit 992fcae into main Mar 12, 2025
15 of 16 checks passed
@surbhigarg92 surbhigarg92 deleted the snapshot_isolation branch March 12, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants