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(Core): add retryFunction in GrpcRequestWrapper::send #5770

Merged
merged 4 commits into from Jan 11, 2023

Conversation

vishwarajanand
Copy link
Contributor

Core now allows adding retryFunction with GrpcRequestWrapper::send. This will be used in Spanner's BeginTransaction to retry for Not_FOUND errors with a custom pattern as followed in other languages.

Code Pointer in TS

A similar change is also being taken by Go, googleapis/google-cloud-go#7163

Change

  1. Adding a retryFunction which takes in an Exception as argument and produces a bool to decide whether to retry or not.

  2. Unit tests to validate cases such as whether retryLimit is still honored and follows the custom logic provided.

ref: b/263775077#comment8

@vishwarajanand vishwarajanand marked this pull request as ready for review January 9, 2023 15:19
@vishwarajanand vishwarajanand requested review from a team as code owners January 9, 2023 15:19
Copy link
Contributor

@bshaffer bshaffer left a comment

Choose a reason for hiding this comment

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

looks good, one minor readability suggestion

Core/tests/Unit/GrpcRequestWrapperTest.php Outdated Show resolved Hide resolved
vishwarajanand and others added 2 commits January 11, 2023 23:34
Co-authored-by: Brent Shaffer <betterbrent@google.com>
@vishwarajanand vishwarajanand merged commit b15f5b1 into googleapis:main Jan 11, 2023
@vishwarajanand vishwarajanand deleted the fix_core_retry_grpc branch January 12, 2023 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants