feat: copy backup API support#1398
Conversation
|
Warning: This pull request is touching the following templated files:
|
| @@ -0,0 +1,408 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
I think this should be reverted?
There was a problem hiding this comment.
I see that the same change was committed 12 days ago to https://github.com/googleapis/java-bigtable/blob/main/google-cloud-bigtable-stats/pom.xml. This was a valid change back then and should be no change now.
There was a problem hiding this comment.
this is the dependency-reduced-pom.xml which gets generated (as opposed to pom.xml) and it should be reverted.
There was a problem hiding this comment.
(separately we should figure out how to exclude this - maybe in .gitignore
There was a problem hiding this comment.
I see, I'll remove this file from the PR. Will figure out how to exclude this and make changes in a different PR, sounds good?
| @@ -0,0 +1,408 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
this is the dependency-reduced-pom.xml which gets generated (as opposed to pom.xml) and it should be reverted.
| * #of(String, String, String, String) of} method | ||
| */ | ||
| public static CopyBackupRequest of(String clusterId, String backupId) { | ||
| CopyBackupRequest request = new CopyBackupRequest(null, null, clusterId, backupId); |
There was a problem hiding this comment.
This doesn't seem to match up - clusterId is the 3rd parameter, but the 3rd parameter should be instanceId per the method signature: https://github.com/googleapis/java-bigtable/pull/1398/files#diff-f22bea44852cc477c88df0651b15890d220ccbfdf7f998d0e97bc6c30d409b8bR65
There was a problem hiding this comment.
The private method is defined as:
private CopyBackupRequest(
@nullable String sourceProjectId,
@nullable String sourceInstanceId,
@nonnull String sourceClusterId,
@nonnull String sourceBackupId);
The 3rd parameter is ClusterId, the order of parameters of the private method and the public ones are different, should I match them?
There was a problem hiding this comment.
got it - in this case, the private method should follow the order of the public ones to avoid confusion (like I just had :)). It will also change the order so the null parameters will go toward the end, which is more typical and expected.
There was a problem hiding this comment.
sg, I'll udpate!
kolea2
left a comment
There was a problem hiding this comment.
lgtm with just some last suggestions on method and variable naming. Thanks for putting this together!
Change CopyBackup method to make it more readable
🤖 I have created a release *beep* *boop* --- ## [2.27.0](https://togithub.com/googleapis/java-bigtable/compare/v2.26.0...v2.27.0) (2023-08-17) ### Features * Copy backup API support ([#1398](https://togithub.com/googleapis/java-bigtable/issues/1398)) ([558a408](https://togithub.com/googleapis/java-bigtable/commit/558a408f5fa0566652df923799cf9f7bc03f7194)) * Publish CopyBackup protos to external customers ([#1883](https://togithub.com/googleapis/java-bigtable/issues/1883)) ([d6e934f](https://togithub.com/googleapis/java-bigtable/commit/d6e934fc71e1c1dd4e13492d2f6c4688b6b0d59d)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.