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.

Sign up for GitHub

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

Jump to bottom

Add context functions to retry and recover #172

Merged
merged 2 commits into from
Nov 12, 2018
Merged

Add context functions to retry and recover #172

merged 2 commits into from
Nov 12, 2018

Conversation

johanbrandhorst
Copy link
Collaborator

@johanbrandhorst johanbrandhorst commented Nov 9, 2018 •
edited
Loading

Fixes #168
Fixes #171

@codecov-io
Copy link

codecov-io commented Nov 9, 2018 •
edited
Loading

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.

bwplotka
bwplotka approved these changes Nov 9, 2018
View reviewed changes
Copy link
Collaborator

@bwplotka bwplotka left a comment •
edited
Loading

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
@johanbrandhorst
retry: add new BackoffFuncContext type
29e1709 
BackoffFuncContext adds the context parameter, allowing
the retry function access to the contents of the parent
context.

Fixes grpc-ecosystem#171
@johanbrandhorst
recovery: add new RecoveryHandlerFuncContext type
f29882f 
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
bwplotka
bwplotka approved these changes Nov 9, 2018
View reviewed changes
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
Reviewers

@hxzhao527 hxzhao527 hxzhao527 left review comments

@bwplotka bwplotka bwplotka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@johanbrandhorst @codecov-io @bwplotka @hxzhao527 @domgreen