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(spanner): add factory functions for instance/database/backup #7321

Merged
merged 2 commits into from
Sep 16, 2021

Conversation

devbww
Copy link
Contributor

@devbww devbww commented Sep 16, 2021

With the move to generated APIs for Spanner admin, the results of
of admin operations now only contain the raw resource strings from
the protobufs. So, it is helpful to be able to build Instance,
Database, and Backup objects directly from those strings.

And to complete the picture, introduce google::cloud::Project,
which will be helpful to encapsulate the slew of "projects/"
concatenations we've accumulated.


This change is Reviewable

With the move to generated APIs for Spanner admin, the results of
of admin operations now only contain the raw resource strings from
the protobufs. So, it is helpful to be able to build `Instance`,
`Database`, and `Backup` objects directly from those strings.

And to complete the picture, introduce `google::cloud::Project`,
which will be helpful to encapsulate the slew of "projects/"
concatenations we've accumulated.
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Sep 16, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 16, 2021
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 3791d9ad2749ddce635a30ddf1e4311474e4403e

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: fc6065ecdcb26337e7bc5b4c99e04e8adcc50914

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link

codecov bot commented Sep 16, 2021

Codecov Report

Merging #7321 (fc6065e) into main (6dde3c6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##             main    #7321    +/-   ##
========================================
  Coverage   93.65%   93.66%            
========================================
  Files        1348     1351     +3     
  Lines      116629   116746   +117     
========================================
+ Hits       109225   109345   +120     
+ Misses       7404     7401     -3     
Impacted Files Coverage Δ
google/cloud/kms_key_name.h 80.00% <ø> (-5.72%) ⬇️
google/cloud/kms_key_name.cc 100.00% <100.00%> (ø)
google/cloud/kms_key_name_test.cc 100.00% <100.00%> (ø)
google/cloud/project.cc 100.00% <100.00%> (ø)
google/cloud/project.h 100.00% <100.00%> (ø)
google/cloud/project_test.cc 100.00% <100.00%> (ø)
google/cloud/spanner/backup.cc 100.00% <100.00%> (ø)
google/cloud/spanner/backup.h 100.00% <100.00%> (ø)
google/cloud/spanner/backup_test.cc 100.00% <100.00%> (ø)
google/cloud/spanner/database.cc 100.00% <100.00%> (ø)
... and 11 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 6dde3c6...fc6065e. Read the comment docs.

@devbww devbww marked this pull request as ready for review September 16, 2021 09:22
@devbww devbww requested a review from a team as a code owner September 16, 2021 09:22
Copy link
Contributor

@coryan coryan left a comment

Choose a reason for hiding this comment

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

Looks like this is mixing a bit of refactoring for KmsKeyName, an other changes. Closely related though.

Curious why the ABI dump needed to change as part of this PR?

@@ -17,7 +17,8 @@

#include "google/cloud/spanner/instance.h"
#include "google/cloud/spanner/version.h"
#include <ostream>
#include "google/cloud/status_or.h"
#include <iosfwd>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@devbww
Copy link
Contributor Author

devbww commented Sep 16, 2021

Looks like this is mixing a bit of refactoring for KmsKeyName, an other changes. Closely related though.

Yeah. I couldn't help but try to make them all more alike.

Curious why the ABI dump needed to change as part of this PR?

It looks like because I changed the existing model for all this from MakeKmsKeyName(std::string) to MakeKmsKeyName(std::string const&). Not really a source incompatibility.

@devbww devbww merged commit 989821c into googleapis:main Sep 16, 2021
@devbww devbww deleted the instance-database-backup-factories branch September 16, 2021 16:31
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 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.

3 participants