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

feat(dbapi): add aborted transactions retry support #168

Merged
merged 5 commits into from Nov 23, 2020

Conversation

IlyaFaer
Copy link
Contributor

@IlyaFaer IlyaFaer commented Nov 12, 2020

@IlyaFaer IlyaFaer added api: spanner Issues related to the googleapis/python-spanner API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Nov 12, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 12, 2020
@IlyaFaer IlyaFaer marked this pull request as ready for review November 13, 2020 07:38
@IlyaFaer IlyaFaer requested a review from a team as a code owner November 13, 2020 07:38
@IlyaFaer IlyaFaer requested a review from c24t November 13, 2020 07:39
Copy link
Contributor

@c24t c24t left a comment

Choose a reason for hiding this comment

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

LGTM with a few minor comments. I checked that this matches the diffs from googleapis/python-spanner-django#544 and googleapis/python-spanner-django#543, see the comments there for more info.

google/cloud/spanner_dbapi/checksum.py Outdated Show resolved Hide resolved
google/cloud/spanner_dbapi/connection.py Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
@@ -0,0 +1,63 @@
# Copyright 2020 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

Another packaging thing to consider: we might want to move all the DBAPI tests under google/cloud/spanner_dbapi/tests/ so the code and tests live in the same package.

Copy link
Contributor

@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@c24t c24t merged commit d59d502 into googleapis:master Nov 23, 2020
@IlyaFaer IlyaFaer deleted the transaction_retry branch November 25, 2020 09:45
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 30, 2020
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. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transaction: on retry, replays should compare checksums of prior numbered statements that succeeded
3 participants