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

fix(storage): disable gax retries for gRPC #9747

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

tritone
Copy link
Contributor

@tritone tritone commented Apr 11, 2024

All gRPC ops are retried as needed at the handwritten layer. Disable gax retries.

All gRPC ops are retried as needed at the handwritten layer.
Disable gax retries.
@tritone tritone requested review from a team as code owners April 11, 2024 03:24
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Apr 11, 2024
@tritone
Copy link
Contributor Author

tritone commented Apr 11, 2024

It looks like this exposed a bunch of conformance test failures; I'm going to fix and update.

@tritone
Copy link
Contributor Author

tritone commented Apr 11, 2024

It looks like this exposed a bunch of conformance test failures; I'm going to fix and update.

Okay, this was only because of a change in how things that are unimplemented in the testbench are handled in the retry test framework. I fixed this and all tests now pass.

Copy link
Contributor

@BrennaEpp BrennaEpp left a comment

Choose a reason for hiding this comment

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

One suggestion that doesn't change the functionality, otherwise LGTM.

We should also look into dropping withGAXOptions.

storage/client_test.go Outdated Show resolved Hide resolved
@tritone tritone added the automerge Merge the pull request once unit tests and other checks pass. label Apr 11, 2024
@gcf-merge-on-green gcf-merge-on-green bot merged commit bbfc0ac into googleapis:main Apr 11, 2024
8 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 11, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants