Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

feat: adding support for spanner request options tags#276

Merged
larkee merged 34 commits intogoogleapis:mainfrom
vi3k6i5:spanner_request_options_2
Sep 29, 2021
Merged

feat: adding support for spanner request options tags#276
larkee merged 34 commits intogoogleapis:mainfrom
vi3k6i5:spanner_request_options_2

Conversation

@vi3k6i5
Copy link
Copy Markdown
Contributor

@vi3k6i5 vi3k6i5 commented Mar 16, 2021

Adding request options to relevant methods and adding unit tests for the same.
Fixes #253 🦕

@vi3k6i5 vi3k6i5 requested review from a team and larkee March 16, 2021 08:07
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Mar 16, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Mar 16, 2021
@vi3k6i5 vi3k6i5 changed the title Spanner request options 2 feat: adding support for spanner request options tags Mar 16, 2021
@vi3k6i5
Copy link
Copy Markdown
Contributor Author

vi3k6i5 commented Mar 23, 2021 •
edited
Loading

System tests are failure is expected because of pending feature release.

Added sleep time of 200 seconds before looking up the tags in db. Just for validation it's okay for now, this will have to be fixed before we merge with mainline.

@vi3k6i5 vi3k6i5 requested review from skuruppu and thiagotnunes April 7, 2021 06:53
@larkee larkee added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 8, 2021
thiagotnunes
thiagotnunes approved these changes Apr 13, 2021
View reviewed changes
larkee
larkee reviewed Apr 14, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Added some comments with more details on transactions.

Additionally, we discussed adding validation but had difficulties due to class inheritance. I think if we add a _read_only attribute to _SnapshotBase and Transaction which is set to True and False respectively, similar to _multi_use, then we should be able to add validation for the request_options.

@skuruppu skuruppu requested a review from larkee July 21, 2021 00:58
@snippet-bot
Copy link
Copy Markdown

snippet-bot bot commented Aug 27, 2021 •
edited
Loading

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@vi3k6i5
Copy link
Copy Markdown
Contributor Author

vi3k6i5 commented Aug 27, 2021

Added some comments with more details on transactions.

Additionally, we discussed adding validation but had difficulties due to class inheritance. I think if we add a _read_only attribute to _SnapshotBase and Transaction which is set to True and False respectively, similar to _multi_use, then we should be able to add validation for the request_options.

Made these changes as well.

larkee
larkee reviewed Aug 31, 2021
View reviewed changes
@vi3k6i5
feat: added support for request options with request tag and transact…
f3aecac 
…ion tag in supported classes
@vi3k6i5
feat: corrected import for RequestOptions
75c91d8
@vi3k6i5
feat: request options added lint corrections
a8e2826
@vi3k6i5
feat: added system test for request tagging
933799a
@vi3k6i5
feat: added annotation to skip request tags validation test while usi…
907c570 
…ng emulator
@vi3k6i5
feat: lint fix
68e77f9
@vi3k6i5
fix: remove request_option from batch
8a659e7
@vi3k6i5
lint: lint fixes
4073c40
@vi3k6i5
refactor: undo changes
a0ff121
@vi3k6i5
refactor: undo changes
5e843d6
@vi3k6i5
refactor: remove test_system file, as it has been removed in master
abd9b8f
@vi3k6i5
refactor: update code to latest changes
e273291
@vi3k6i5
feat: added support for request options with request tag and transact…
1bdd637 
…ion tag in supported classes
@vi3k6i5
feat: corrected import for RequestOptions
edcced6
@vi3k6i5
fix: add transaction_tag test for transaction_tag set in transaction …
c99e60d 
…class
@vi3k6i5 vi3k6i5 requested a review from larkee September 10, 2021 08:17
@skuruppu
Copy link
Copy Markdown
Contributor

skuruppu commented Sep 24, 2021

@thiagotnunes I ran out of time to review this before I go on vacation. Would you please be able to review and approve on Monday?

@larkee larkee removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 29, 2021
@larkee larkee merged commit e16f376 into googleapis:main Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@skuruppu skuruppu Awaiting requested review from skuruppu

@thiagotnunes thiagotnunes Awaiting requested review from thiagotnunes

1 more reviewer

@larkee larkee larkee approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/python-spanner API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add tagging support on database operations such as reads, queries and transactions.

4 participants

@vi3k6i5 @skuruppu @thiagotnunes @larkee