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

Add context functions to retry and recover #172

Conversation

johanbrandhorst
Copy link
Collaborator

@johanbrandhorst johanbrandhorst commented Nov 9, 2018

Fixes #168
Fixes #171

@codecov-io
Copy link

codecov-io commented Nov 9, 2018

Codecov Report

Merging #172 into master will decrease coverage by 0.56%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
- Coverage   73.47%   72.91%   -0.57%     
==========================================
  Files          36       36              
  Lines        1327     1340      +13     
==========================================
+ Hits          975      977       +2     
- Misses        303      314      +11     
  Partials       49       49
Impacted Files Coverage Δ
recovery/interceptors.go 100% <100%> (ø) ⬆️
retry/retry.go 76.11% <100%> (-2.23%) ⬇️
recovery/options.go 78.57% <80%> (-21.43%) ⬇️
retry/options.go 80.48% <87.5%> (-7.4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 498ae20...f29882f. Read the comment docs.

Copy link
Collaborator

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, some comments could be more clear maybe, but not a blocker!

recovery/interceptors.go Show resolved Hide resolved
retry/options.go Show resolved Hide resolved
BackoffFuncContext adds the context parameter, allowing
the retry function access to the contents of the parent
context.

Fixes grpc-ecosystem#171
Allows the user to configure a custom recovery handler
that has access to the request scoped values in the
context.

Fixes grpc-ecosystem#168
@johanbrandhorst johanbrandhorst force-pushed the add-context-retry-recovery-parameters branch from a39880c to f29882f Compare November 9, 2018 15:02
Copy link
Collaborator

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice, thanks for extra comments. Let's ping @domgreen to merge this.

@johanbrandhorst
Copy link
Collaborator Author

Bump @domgreen 😁

@domgreen domgreen merged commit 3304cc8 into grpc-ecosystem:master Nov 12, 2018
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.

5 participants