Skip to content

feat(storage): add retry config to BucketHandle#5170

Merged
BrennaEpp merged 10 commits intogoogleapis:mainfrom
BrennaEpp:bucket-retry
Dec 2, 2021
Merged

feat(storage): add retry config to BucketHandle#5170
BrennaEpp merged 10 commits intogoogleapis:mainfrom
BrennaEpp:bucket-retry

Conversation

@BrennaEpp
Copy link
Copy Markdown
Contributor

@BrennaEpp BrennaEpp commented Nov 26, 2021

  • Adds Retryer configurability to BucketHandle
  • Adds retrying to BucketHandle.Update, including defaulting to retry only when idempotency conditions are present
  • Adds retry config to all direct methods on BucketHandle
  • Adds integration test for retry configs

Bucket config will merge with object config, with the object's config overriding the options it sets.

@BrennaEpp BrennaEpp requested review from a team November 26, 2021 05:15
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 26, 2021
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Nov 26, 2021
Comment thread storage/bucket.go
Comment thread storage/bucket.go
@BrennaEpp BrennaEpp requested a review from tritone December 1, 2021 03:10
Comment thread storage/storage.go Outdated
Comment thread storage/integration_test.go Outdated
return transport.Creds(ctx, opts...)
}

func TestIntegration_RetryConfig(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like this test is a bit hacky since it depends on an unrealistic shouldRetry method. Do we really need an integration test? I feel like it's probably sufficient to just to write a unit test that verifies that the configs are what we expect them to be, if options are set on either or both of the BucketHandle and ObjectHandle.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point. switched it to unit test

Comment thread storage/storage.go Outdated
@BrennaEpp BrennaEpp requested a review from tritone December 1, 2021 22:37
@BrennaEpp BrennaEpp enabled auto-merge (squash) December 2, 2021 19:56
@BrennaEpp BrennaEpp merged commit b2b5476 into googleapis:main Dec 2, 2021
@BrennaEpp BrennaEpp deleted the bucket-retry branch February 8, 2023 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants