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

@thiagotnunes thiagotnunes commented Oct 21, 2020

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 as code owners Oct 21, 2020
@google-cla google-cla bot added the cla: yes 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 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

@mayurkale22 mayurkale22 Oct 21, 2020

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
Contributor

@olavloite olavloite Oct 21, 2020

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

@mayurkale22 mayurkale22 Oct 21, 2020

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 label Oct 21, 2020
@thiagotnunes thiagotnunes merged commit 659719d into googleapis:master Oct 22, 2020
19 of 21 checks passed
@thiagotnunes thiagotnunes deleted the write-with-options branch Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants