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: introduce TransactionOptions and UpdateOptions #716

Merged
merged 2 commits into from Dec 15, 2020

Conversation

olavloite
Copy link
Collaborator

Adds TransactionOptions and UpdateOptions for read/write transactions and statements. These can be used in the future to specify options to affect how transactions and statements will be executed.

These changes can be used as the basis for #676 and #576 to ensure both changes use the same approach, and to prevent enormous merge conflicts.

Adds TransactionOptions and UpdateOptions for read/write transactions and statements.
These can be used in the future to specify options to affect how transactions and
statements will be executed.
@olavloite olavloite requested review from a team as code owners December 11, 2020 10:40
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 11, 2020
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Dec 11, 2020
Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

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

I like this PR, this will certainly simplify #676 and #576. Thank you so much for doing this!

@@ -340,10 +343,11 @@ public void run() {
return res;
}

TransactionContextImpl newTransaction() {
TransactionContextImpl newTransaction(Options options) {
Copy link
Member

Choose a reason for hiding this comment

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

Can this be just TransactionOption instead of Options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be possible (assuming you mean TransactionOption...), but I would prefer to keep this as the Options object. This makes it a required argument, making it less likely to be missed by accident as it will cause a compile error if you don't supply an Options object. If we change it to TransactionOption... it would not cause a compile time error if we forget to pass in the options somewhere.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, makes sense.

@mayurkale22 mayurkale22 mentioned this pull request Dec 11, 2020
1 task
@codecov
Copy link

codecov bot commented Dec 11, 2020

Codecov Report

Merging #716 (ae2cf93) into master (f14f7c9) will increase coverage by 0.08%.
The diff coverage is 88.34%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #716      +/-   ##
============================================
+ Coverage     85.03%   85.12%   +0.08%     
- Complexity     2553     2561       +8     
============================================
  Files           142      142              
  Lines         13930    13952      +22     
  Branches       1326     1331       +5     
============================================
+ Hits          11846    11877      +31     
+ Misses         1527     1515      -12     
- Partials        557      560       +3     
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/google/cloud/spanner/Options.java 85.48% <40.00%> (-3.99%) 67.00 <2.00> (+2.00) ⬇️
...oogle/cloud/spanner/PartitionedDmlTransaction.java 82.60% <83.33%> (+0.19%) 15.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java 89.02% <91.17%> (+0.48%) 71.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionImpl.java 85.79% <92.30%> (+1.77%) 30.00 <4.00> (+1.00)
...om/google/cloud/spanner/TransactionRunnerImpl.java 84.36% <93.75%> (+0.09%) 9.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java 86.76% <100.00%> (ø) 47.00 <0.00> (ø)
...gle/cloud/spanner/AsyncTransactionManagerImpl.java 71.83% <100.00%> (+0.40%) 12.00 <0.00> (ø)
...a/com/google/cloud/spanner/DatabaseClientImpl.java 84.46% <100.00%> (+3.51%) 21.00 <2.00> (+2.00)
...ud/spanner/SessionPoolAsyncTransactionManager.java 85.71% <100.00%> (+0.34%) 11.00 <0.00> (ø)
...m/google/cloud/spanner/TransactionManagerImpl.java 88.88% <100.00%> (+0.20%) 20.00 <1.00> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f14f7c9...ae2cf93. Read the comment docs.

Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

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

LGTM

@olavloite
Copy link
Collaborator Author

@skuruppu Do you want to weigh in on this as well? It is technically a (binary) breaking change as we are adding vararg arguments to a number of public interface methods. Existing client application code does not need to be changed, but it breaks binary compatibility, so it needs to be recompiled.

@skuruppu
Copy link
Contributor

I think previously, we haven't marked changes that break binary compatibility as breaking changes so this is fine.

@olavloite olavloite merged commit 5c96fab into master Dec 15, 2020
@olavloite olavloite deleted the introduce-transaction-options branch December 15, 2020 07:20
thiagotnunes pushed a commit that referenced this pull request May 6, 2021
* feat: introduce TransactionOptions and UpdateOptions

Adds TransactionOptions and UpdateOptions for read/write transactions and statements.
These can be used in the future to specify options to affect how transactions and
statements will be executed.

* test: add options tests
ansh0l pushed a commit to ansh0l/java-spanner that referenced this pull request Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by
cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will
update the corresponding PR to depend on the newer version of go-genproto, and
assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot
will not create any more regeneration PRs. If all regen PRs are closed,
gapicgen will create a new set of regeneration PRs once per night.

If you have been assigned to review this PR, please:

- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship. That will prompt
genbot to assign reviewers to the google-cloud-go PR.

Corresponding google-cloud-go PR: googleapis/google-cloud-go#5112

Changes:

feat(texttospeech): update v1beta1 proto
  PiperOrigin-RevId: 409139454
  Source-Link: googleapis/googleapis@7c10623

chore: regenerate API index

  Source-Link: googleapis/googleapis@753ad85

chore(compute): Add C# generation for Compute v1

  Source-Link: googleapis/googleapis@05a6a82

feat(dialogflow/cx): allow setting custom CA for generic webhooks
  PiperOrigin-RevId: 408995680
  Source-Link: googleapis/googleapis@76f7f48

fix!(networkconnectivity): Mark a couple networkconnectivity API fields as required, to match implemented behavior
  PiperOrigin-RevId: 408969147
  Source-Link: googleapis/googleapis@cc003a4

chore(aiplatform): Add partial C# generation to AI Platform
  PiperOrigin-RevId: 408964828
  Source-Link: googleapis/googleapis@895ad28

chore(binaryauthorization): Fix SystemPolicy service name for BinaryAuthorization V1beta1
  PiperOrigin-RevId: 408764899
  Source-Link: googleapis/googleapis@66878ca

chore(iam/credentials): Update iam/credentials BUILD.bazel package name to google-cloud-iam chore(python): Update iam/credentials namespace to google.cloud.iam_credentials
  PiperOrigin-RevId: 408717007
  Source-Link: googleapis/googleapis@171d777

docs: fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 408715661
  Source-Link: googleapis/googleapis@5bb0518

chore(container): Update container BUILD.bazel package name to google-cloud-container
  chore(python): Update namespace for container API to google.cloud

  PiperOrigin-RevId: 408673815
  Source-Link: googleapis/googleapis@c08b149

chore: Add C# generation rules
  PiperOrigin-RevId: 408640431
  Source-Link: googleapis/googleapis@b32a324

docs(datacatalog): Improved formatting
  PiperOrigin-RevId: 408569512
  Source-Link: googleapis/googleapis@c7b3bd0

docs(securitycenter): fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 408564402
  Source-Link: googleapis/googleapis@3020af5

feat(compute): Move compute.v1 from googleapis-discovery to googleapis (googleapis#675)
  * feat: Move compute.v1 from googleapis-discovery to googleapis

  The WORKSPACE file changes are expected to be overridden by the next sync, and it is ok, as the synce'd copy will contain the same changes.

  * feat: sync latest compute.proto files from googleapis-discovery
  Source-Link: googleapis/googleapis@691a18b

docs(samples): add example tags to generated samples
  PiperOrigin-RevId: 408439482
  Source-Link: googleapis/googleapis@b9f6184

chore(aiplatform): update Java and Python dependencies
  PiperOrigin-RevId: 408420890
  Source-Link: googleapis/googleapis@2921f9f

feat: update gapic-generator-csharp to 1.3.15 feat: update rules_gapic to 0.10.0
  Committer: @viacheslav-rostovtsev
  PiperOrigin-RevId: 408407783
  Source-Link: googleapis/googleapis@1bd869b

docs(gaming): fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 408383287
  Source-Link: googleapis/googleapis@2aec4d0

feat(securitycenter): Added resource type and display_name field to the FindingResult, and supported them in the filter for ListFindings and GroupFindings. Also added display_name to the resource which is surfaced in NotificationMessage
  PiperOrigin-RevId: 408362247
  Source-Link: googleapis/googleapis@4d71c45

feat(redis): [Cloud Memorystore for Redis] Support Multiple Read Replicas when creating Instance
  PiperOrigin-RevId: 408360324
  Source-Link: googleapis/googleapis@78eb8a2

feat(redis): [Cloud Memorystore for Redis] Support Multiple Read Replicas when creating Instance
  PiperOrigin-RevId: 408360267
  Source-Link: googleapis/googleapis@8625cf0

docs(storage/internal): Add comments to GCS gRPC API proto spec to describe how naming works
  PiperOrigin-RevId: 408352508
  Source-Link: googleapis/googleapis@be4be3d

chore: regenerate API index

  Source-Link: googleapis/googleapis@9974d09

feat(binaryauthorization): add new admission rule types to Policy feat: update SignatureAlgorithm enum to match algorithm names in KMS feat: add SystemPolicyV1Beta1 service
  PiperOrigin-RevId: 408346628
  Source-Link: googleapis/googleapis@3dfbdc3

feat(datacatalog): Added BigQueryDateShardedSpec.latest_shard_resource field feat: Added SearchCatalogResult.display_name field feat: Added SearchCatalogResult.description field
  PiperOrigin-RevId: 408274419
  Source-Link: googleapis/googleapis@cbba92c

docs(resourcemanager): fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 407854413
  Source-Link: googleapis/googleapis@f81b7bd

chore: try to fix the mistaken-pull-closer config
  PiperOrigin-RevId: 407849101
  Source-Link: googleapis/googleapis@9e6c8c6

docs(ruby): Add simple code examples for all RPC methods
  PiperOrigin-RevId: 407847849
  Source-Link: googleapis/googleapis@1112610

feat(workflows/executions): add a stack_trace field to the Error messages specifying where the error occured feat: add call_log_level field to Execution messages doc: clarify requirement to escape strings within JSON arguments
  Update the Execution proto with stack_trace field which is populated on output if an error occurs, and the call_log_level field can be specified on input to request that function calls and/or errors for the given execution be logged to Cloud Logging.

  PiperOrigin-RevId: 407842136
  Source-Link: googleapis/googleapis@cd48c16

feat(dialogflow): added support to configure security settings, language code and time zone on conversation profile
  PiperOrigin-RevId: 407663596
  Source-Link: googleapis/googleapis@f9acb37

chore: generate java files for apps/script/type protos
  PiperOrigin-RevId: 407655547
  Source-Link: googleapis/googleapis@5946b7a

feat(functions): Secret Manager integration fields 'secret_environment_variables' and 'secret_volumes' added feat: CMEK integration fields 'kms_key_name' and 'docker_repository' added
  PiperOrigin-RevId: 407654258
  Source-Link: googleapis/googleapis@09b2576

chore: regenerate API index

  Source-Link: googleapis/googleapis@ddd6637

feat(contactcenterinsights): Add ability to update phrase matchers feat: Add issue model stats to time series feat: Add display name to issue model stats
  PiperOrigin-RevId: 407647313
  Source-Link: googleapis/googleapis@33fd29d
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
…2.0 (googleapis#716)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:libraries-bom](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java) | `24.1.2` -> `24.2.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/24.2.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/24.2.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/24.2.0/compatibility-slim/24.1.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/24.2.0/confidence-slim/24.1.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-spanner-jdbc).
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/java-spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants