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: adds options to the write operations #531

Merged
merged 3 commits into from Oct 22, 2020

Conversation

thiagotnunes
Copy link
Contributor

Adds the possibility of adding options to the write operations and encapsulates the write response into its own class so that we can augment the response with more fields than the commit timestamp.

At the moment, the new methods just delegate to the existing write methods. Once we add write option implementations we should change the definition of the added methods.

@thiagotnunes thiagotnunes requested review from a team as code owners October 21, 2020 00:27
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 21, 2020
@thiagotnunes thiagotnunes changed the title feat!: adds options to the write operations feat: adds options to the write operations Oct 21, 2020
Adds the possibility of adding options to the write operations and
encapsulates the write response into it's own class so that we can
augment the response with more fields than the commit timestamp.
@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #531 into master will decrease coverage by 0.14%.
The diff coverage is 3.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #531      +/-   ##
============================================
- Coverage     84.03%   83.88%   -0.15%     
- Complexity     2537     2540       +3     
============================================
  Files           140      141       +1     
  Lines         13891    13918      +27     
  Branches       1329     1330       +1     
============================================
+ Hits          11673    11675       +2     
- Misses         1674     1699      +25     
  Partials        544      544              
Impacted Files Coverage Δ Complexity Δ
.../java/com/google/cloud/spanner/CommitResponse.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...a/com/google/cloud/spanner/DatabaseClientImpl.java 82.30% <0.00%> (-3.03%) 25.00 <0.00> (+3.00) ⬇️
...rc/main/java/com/google/cloud/spanner/Options.java 89.47% <ø> (ø) 65.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java 86.11% <0.00%> (-0.60%) 110.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionImpl.java 84.61% <20.00%> (-2.06%) 30.00 <0.00> (ø)
...m/google/cloud/spanner/spi/v1/GapicSpannerRpc.java 82.90% <0.00%> (+0.28%) 81.00% <0.00%> (ø%)

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 777f5fc...902915f. Read the comment docs.

Updates WriteOption to TransactionOption and changes the WriteResponse
to CommitResponse.
*
* @return a response with the timestamp at which the write was committed
*/
CommitResponse writeWithOptions(Iterable<Mutation> mutations, TransactionOption... options)
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about WriteOption in place of TransactionOption?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thiago and I had a short discussion about this, and we think TransactionOption is better in this case, as we intend to also use this for options that we will need for normal read/write transactions.

Copy link
Member

Choose a reason for hiding this comment

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

Ok sgtm

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Oct 21, 2020
@thiagotnunes thiagotnunes merged commit 659719d into googleapis:master Oct 22, 2020
@thiagotnunes thiagotnunes deleted the write-with-options branch October 22, 2020 23:17
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#3702

Changes:

chore(longrunning): Operations proto comment cleanup
  Committer: @miraleung
  PiperOrigin-RevId: 357075713
  Source-Link: googleapis/googleapis@c059c46

docs(billing/budgets): Update rules for currency_code in budget_amount.
  PiperOrigin-RevId: 357051517
  Source-Link: googleapis/googleapis@f3a60f6
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
This PR was generated using Autosynth. 🌈


<details><summary>Log from Synthtool</summary>

```
2021-07-07 14:59:54,920 synthtool [DEBUG] > Executing /root/.cache/synthtool/java-spanner-jdbc/.github/readme/synth.py.
On branch autosynth-readme
nothing to commit, working tree clean
2021-07-07 14:59:56,130 synthtool [DEBUG] > Wrote metadata to .github/readme/synth.metadata/synth.metadata.

```
</details>

Full log will be available here:
https://source.cloud.google.com/results/invocations/ba544d15-79a9-4161-b4d1-03f316693e40/targets

- [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.)
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

3 participants