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.

Sign up for GitHub

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

Jump to bottom

feat(spanner): add support for max commit delay #13562

Merged
merged 17 commits into from Feb 29, 2024
Merged

feat(spanner): add support for max commit delay #13562

merged 17 commits into from Feb 29, 2024

Conversation

nginsberg-google
Copy link
Contributor

@nginsberg-google nginsberg-google commented Feb 7, 2024 •
edited by coryan

This change is Reviewable

@nginsberg-google nginsberg-google requested a review from a team as a code owner February 7, 2024 01:20
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Feb 7, 2024
@devbww
Copy link
Collaborator

devbww commented Feb 7, 2024

/gcbrun

@codecov Codecov
Copy link

codecov bot commented Feb 7, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.30%. Comparing base (8e7f422) to head (4c88e5d).
Report is 6 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #13562   +/-   ##
=======================================
  Coverage   93.30%   93.30%           
=======================================
  Files        2228     2228           
  Lines      193366   193421   +55     
=======================================
+ Hits       180411   180467   +56     
+ Misses      12955    12954    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coryan coryan changed the title feat(Spanner): Add support for max commit delay. feat(Spanner): add support for max commit delay Feb 7, 2024
@coryan coryan changed the title feat(Spanner): add support for max commit delay feat(spanner): add support for max commit delay Feb 7, 2024
@nginsberg-google
Copy link
Contributor Author

@devbww - Looks like there was a small gap in the unit testing - this is now fixed. PTAL.

@nginsberg-google
Copy link
Contributor Author

/gcbrun

1 similar comment
@dbolduc
Copy link
Member

dbolduc commented Feb 7, 2024

/gcbrun

devbww
devbww reviewed Feb 12, 2024
View reviewed changes
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.cc Show resolved Hide resolved
google/cloud/spanner/commit_options.cc Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/options.h Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
nginsberg-google
nginsberg-google commented Feb 13, 2024
View reviewed changes
Copy link
Contributor Author

@nginsberg-google nginsberg-google left a comment

Choose a reason for hiding this comment

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

Thank you very much for the detailed review!

Everything should be addressed now. PTAL.

google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/client_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/options.h Outdated Show resolved Hide resolved
@nginsberg-google
Copy link
Contributor Author

/gcbrun

1 similar comment
@devbww
Copy link
Collaborator

devbww commented Feb 13, 2024

/gcbrun

devbww
devbww reviewed Feb 13, 2024
View reviewed changes
google/cloud/spanner/client_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/options.h Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl.cc Outdated Show resolved Hide resolved
@devbww
Copy link
Collaborator

devbww commented Feb 14, 2024

/gcbrun

devbww
devbww reviewed Feb 14, 2024
View reviewed changes
google/cloud/spanner/client_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl.cc Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
@devbww
Copy link
Collaborator

devbww commented Feb 29, 2024

Navarre: FYI ... We'll be cutting a google-cloud-cpp release on March 4, and then not again until April 12, so if you want this to appear in a numbered/tagged release before April 12, it will need to be merged in the next few days.

@nginsberg-google
Copy link
Contributor Author

nginsberg-google commented Feb 29, 2024 via email

nginsberg-google
nginsberg-google commented Feb 29, 2024
View reviewed changes
Copy link
Contributor Author

@nginsberg-google nginsberg-google left a comment

Choose a reason for hiding this comment

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

@devbww - I've addressed the remaining comments. Sorry again for the delay.

google/cloud/spanner/client_test.cc Outdated Show resolved Hide resolved
google/cloud/spanner/commit_options.h Outdated Show resolved Hide resolved
google/cloud/spanner/internal/connection_impl.cc Outdated Show resolved Hide resolved
@devbww
Copy link
Collaborator

devbww commented Feb 29, 2024

/gcbrun

@devbww
Copy link
Collaborator

devbww commented Feb 29, 2024

/gcbrun

@devbww
Copy link
Collaborator

devbww commented Feb 29, 2024

/gcbrun

devbww
devbww approved these changes Feb 29, 2024
View reviewed changes
Copy link
Collaborator

@devbww devbww left a comment

Choose a reason for hiding this comment

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

Thanks.

@devbww devbww merged commit 9645780 into googleapis:main Feb 29, 2024
62 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devbww devbww devbww approved these changes

Assignees
No one assigned
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nginsberg-google @devbww @dbolduc