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): allow restore backup to different instance (#3489) #4014

Merged
merged 22 commits into from May 3, 2021

Conversation

kolea2
Copy link
Contributor

@kolea2 kolea2 commented Apr 27, 2021

  • feat: restore table to a different instance

  • add check that table restored

  • review fixes

  • change instance config zone

* feat: restore table to a different instance

* add check that table restored

* review fixes

* change instance config zone
@kolea2 kolea2 requested review from crwilcox, tritone and a team as code owners April 27, 2021 14:20
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Apr 27, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021
…ackup-different-instance

# Conflicts:
#	bigtable/admin.go
@kolea2
Copy link
Contributor Author

kolea2 commented Apr 27, 2021

@liubonan fyi

@kolea2 kolea2 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 27, 2021
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Show resolved Hide resolved
bigtable/integration_test.go Show resolved Hide resolved
@tbpg
Copy link
Contributor

tbpg commented Apr 28, 2021

https://github.com/golang/go/wiki/TestComments#keep-going is relevant to using t.Fatalf vs t.Errorf.

@kolea2
Copy link
Contributor Author

kolea2 commented Apr 30, 2021

@tbpg @tritone I believe all comments have been addressed. Thanks for the review!

@kolea2 kolea2 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 30, 2021
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

Generally looks good, a few more testing Q's

bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Show resolved Hide resolved
Copy link
Contributor

@tbpg tbpg left a comment

Choose a reason for hiding this comment

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

A couple minor comments. Overall, looks good.

bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/integration_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tbpg tbpg left a comment

Choose a reason for hiding this comment

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

A couple minor comment comments. Looks good.

bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
kolea2 and others added 3 commits May 3, 2021 14:45
Co-authored-by: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com>
Co-authored-by: Tyler Bui-Palsulich <26876514+tbpg@users.noreply.github.com>
@kolea2
Copy link
Contributor Author

kolea2 commented May 3, 2021

Thank you @tbpg @tritone !

@kolea2 kolea2 merged commit b08b265 into master May 3, 2021
@kolea2 kolea2 deleted the backup-different-instance branch May 3, 2021 19:01
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants