-
Notifications
You must be signed in to change notification settings - Fork 20
feat: Support for Spanner ReadLockMode #171
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
Conversation
aandreassa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment on documentation. Could we add an integration test just to make sure the server is accepting the new argument?
I have added in a |
aandreassa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits!
Also, I am OK with adding a single integration test that sets fields in transaction options. Would it be difficult to do it in this PR?
It should be helpful to double check that we are sending the correct format for the request.
chore: Fix lint issues chore: Address review comments
bdb4a9f to
20139bd
Compare
aandreassa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
This PR adds in the ability to set ReadLockMode as a nested option under ReadWrite transaction options.
TESTED=All acceptance and unit tests pass