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): add update table metadata support #6746

Merged

Conversation

kimihrr
Copy link
Contributor

@kimihrr kimihrr commented Sep 26, 2022

  • Add DeletionProtection field to TableConf and TableInfo
  • Add UpdateTable function to change the value of DeletionProtection
  • Make sure that only DeletionProtection can be updated through UpdateTable
  • Add mockTableAdminClock and setupTableClient to support tests for Table admin APIs
  • Add tests for updateTable
  • createTable changes to support DeletionProtection will be added in the next pull request

@kimihrr kimihrr requested review from enocom, telpirion and a team as code owners September 26, 2022 22:26
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the Bigtable API. labels Sep 26, 2022
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
Copy link

@kevinsi4508 kevinsi4508 left a comment

Choose a reason for hiding this comment

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

I have not looked at the test yet.

bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
codyoss and others added 13 commits September 28, 2022 20:20
This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#911

Changes:

feat(aiplatform): add model_source_info to Model in aiplatform v1beta1 model.proto
  PiperOrigin-RevId: 476411826
  Source-Link: googleapis/googleapis@72f0faa

docs(language): fix docstring formatting
  Committer: @parthea
  PiperOrigin-RevId: 476410563
  Source-Link: googleapis/googleapis@7f579ee

feat(aiplatform): add model_source_info to Model in aiplatform v1 model.proto
  PiperOrigin-RevId: 476193748
  Source-Link: googleapis/googleapis@a7f3890

chore(retail): revert removal of LRO mixin
  PiperOrigin-RevId: 476177134
  Source-Link: googleapis/googleapis@174b42d

fix(dialogflow/cx): revert removal of LRO mixin
  PiperOrigin-RevId: 476177109
  Source-Link: googleapis/googleapis@652c4c1

feat(iam): remove ListApplicablePolicies
  PiperOrigin-RevId: 475955031
  Source-Link: googleapis/googleapis@65376f4

feat(bigquery/analyticshub): rename nodejs analyticshub library package name
  PiperOrigin-RevId: 475881872
  Source-Link: googleapis/googleapis@8933d4b

chore(dataplex): remove duplicated IAM mixin for Node.js library
  PiperOrigin-RevId: 475855448
  Source-Link: googleapis/googleapis@b7c271c

feat(dataproc): add support for Dataproc metric configuration
  Committer: @AkshatBhargava
  PiperOrigin-RevId: 475750057
  Source-Link: googleapis/googleapis@30517fd

chore(bigquery/storage): update gapic-generator-python-1.4.4 with unit tests generation fixes
  PiperOrigin-RevId: 475683078
  Source-Link: googleapis/googleapis@df791ce

feat(language): Add support for V1 and V2 classification models for the V1Beta2 API
  PiperOrigin-RevId: 475604619
  Source-Link: googleapis/googleapis@044a15c

feat(language): Add support for V1 and V2 classification models for the V1 API
  PiperOrigin-RevId: 475599241
  Source-Link: googleapis/googleapis@05b99f9

feat(aiplatform): add timestamp_outside_retention_rows_count to ImportFeatureValuesResponse and ImportFeatureValuesOperationMetadata in aiplatform v1 featurestore_service.proto feat: add RemoveContextChildren rpc to aiplatform v1 metadata_service.proto feat: add order_by to ListArtifactsRequest, ListContextsRequest, and ListExecutionsRequest in aiplatform v1 metadata_service.proto
  PiperOrigin-RevId: 475580702
  Source-Link: googleapis/googleapis@af65a19

feat(aiplatform): add timestamp_outside_retention_rows_count to ImportFeatureValuesResponse and ImportFeatureValuesOperationMetadata in aiplatform v1beta1 featurestore_service.proto feat: add RemoveContextChildren rpc to aiplatform v1beta1 metadata_service.proto feat: add order_by to ListArtifactsRequest, ListContextsRequest, and ListExecutionsRequest in aiplatform v1beta1 metadata_service.proto feat: add InputArtifact to RuntimeConfig in aiplatform v1beta1 pipeline_job.proto feat: add read_mask to ListPipelineJobsRequest in aiplatform v1beta1 pipeline_service.proto feat: add TransferLearningConfig in aiplatform v1beta1 study.proto
  PiperOrigin-RevId: 475580307
  Source-Link: googleapis/googleapis@dbc83bd

chore(dataform): update gapic-generator-python to 1.4.3 with test fixes
  PiperOrigin-RevId: 475399737
  Source-Link: googleapis/googleapis@4c4a9a2

feat(asset): Add client library support for AssetService v1 SavedQuery APIs Committer: jeffreyai@
  PiperOrigin-RevId: 475366952
  Source-Link: googleapis/googleapis@7428dad
This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#912
…pis#6695)

This PR adds support for automatic retry of failed appends.  Failures are evaluated both from the perspective of any receive errors getting the response, as well as any response that may be embedded in response from the service.  Previous behavior was to simply re-attempt failures when issuing the append.

This PR also adds a new WriterOption (`DisableWriteRetries(disable bool)`) to control this behavior (default is to have retries enabled).

For users sensitive to write duplication, this PR also exposes a new TotalAttempts method on the AppendResult, which will indicate the total number of times this write was attempted.

This also tries to clean up retries in general a bit more.  The generated client will already retry unary RPCs, subject to the [service config](https://github.com/googleapis/googleapis/blob/master/google/cloud/bigquery/storage/v1/bigquerystorage_grpc_service_config.json) present when generating the storage API.

We specifically clarify/introduce two additional kinds of retries above that: a unary retry and a stateless retry.

The unary retry is used to (re)open the underlying bidi network connection which appends are sent upon, as we want to be resilient to reconnection.  The unary retry is effectively stateful for the operation of reopening the connection, and thus uses a gax-based backoff that backs off with increasing intervals.

The stateless retry is used when processing the responses returning from the backend on the bidi stream, where backing off incrementally doesn't make sense.  Instead, we use a base backoff and jitter, and for cases where a more severe backoff is warranted (throughput exhaustion) we use a multiplication factor.  The intent here is to provide backpressure which will eventually saturate the append queue and cause blocking/rejection of writes until the backlog recovers.
Adds retry config to grpcReader and grpcWriter types so that they can be propagated to operations as needed. Ensure that the configured values are used for all retry calls, and that UserProject is consistently passed through as well.
Add dual transport tests for HMAC keys and for PostPolicyV4.

One PostPolicyV4 test creates a second client with custom key behavior. I split this test into two separate tests, one of which fails for gRPC. I left this as a skip for now since it will require some digging in the constructor creds logic to fix.

I also cleaned up some layers of indirection around the various testConfig client methods, and now the multiTransportTest function can take client options to pass down to the client constructors which should make it easier to clean up some other tests which have custom client constructor logic.
…ogleapis#6761)

Per guidance from backend team, include FailedPrecondition as part
of the allowed retry.
Copy link
Contributor

@telpirion telpirion left a comment

Choose a reason for hiding this comment

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

I think this feature probably needs an integration test, too.

bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Outdated Show resolved Hide resolved
bigtable/admin.go Show resolved Hide resolved
bigtable/admin.go Show resolved Hide resolved
bigtable/admin_test.go Outdated Show resolved Hide resolved
@telpirion telpirion added the automerge Merge the pull request once unit tests and other checks pass. label Oct 5, 2022
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 6, 2022
@telpirion telpirion added the automerge Merge the pull request once unit tests and other checks pass. label Oct 6, 2022
@telpirion telpirion enabled auto-merge (squash) October 6, 2022 19:44
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 6, 2022
auto-merge was automatically disabled October 7, 2022 17:13

Head branch was pushed to by a user without write access

@telpirion telpirion added the automerge Merge the pull request once unit tests and other checks pass. label Oct 7, 2022
@telpirion telpirion enabled auto-merge (squash) October 7, 2022 18:51
@telpirion telpirion merged commit f19ffad into googleapis:main Oct 7, 2022
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 7, 2022
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: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.