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(bigtable): Support copy backup in admin client #9005

Merged
merged 7 commits into from
Dec 1, 2023

Conversation

bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Nov 14, 2023

This PR adds CopyBackup method to admin client using which existing backups can be copied to a same or different project.

@bhshkh bhshkh requested review from a team as code owners November 14, 2023 18:29
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Nov 14, 2023
@bhshkh bhshkh requested a review from enocom November 14, 2023 18:29
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Nov 14, 2023
@bhshkh bhshkh requested review from gkevinzheng and removed request for enocom November 16, 2023 21:58
Copy link
Contributor

@gkevinzheng gkevinzheng left a comment

Choose a reason for hiding this comment

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

LGTM

@bhshkh bhshkh enabled auto-merge (squash) November 29, 2023 18:58
CONTRIBUTING.md Outdated Show resolved Hide resolved
bigtable/export_test.go Outdated Show resolved Hide resolved
bigtable/export_test.go Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
@bhshkh bhshkh requested a review from kolea2 November 29, 2023 21:27
Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

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

a few small nits, but overall LGTM!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
internal/kokoro/continuous.sh Show resolved Hide resolved
bigtable/admin.go Show resolved Hide resolved
@@ -132,6 +136,7 @@ project's service account.
- `GCLOUD_TESTS_GOLANG_FIRESTORE_KEY`: The path to the JSON key file of the
Firestore project's service account.
- `GCLOUD_TESTS_API_KEY`: API key for using the Translate API created above.
- `GCLOUD_TESTS_GOLANG_SECONDARY_BIGTABLE_PROJECT_ID`: Developers Console project's ID (e.g. doorway-cliff-677) for Bigtable optional secondary project. This can be same as Firestore project or any project other than the general project.
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't an env var needed for a Bigtable API key, similar to the other projects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only Translate API is using API key for running tests.
None of the Bigtable tests use the API key for authentication.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant JSON key file.

Copy link
Member

Choose a reason for hiding this comment

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

It looks to me like the Bigtable test might now depend on GCLOUD_TESTS_GOLANG_FIRESTORE_KEY, is that correct?

@bhshkh bhshkh merged commit 834c47f into googleapis:main Dec 1, 2023
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants