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

Introduce public API for Hedging Resilience Strategy #1160

Merged
merged 4 commits into from
Apr 26, 2023
Merged

Conversation

martintmk
Copy link
Contributor

@martintmk martintmk commented Apr 26, 2023

The issue or feature being addressed

Contributes to #876

Details on the issue fix or feature implementation

This PR introduces the public API shape for the hedging strategy. The implementation of hedging strategy to be done in follow-up.

API usage:

// Generic hedging for handling of single result type
builder.AddHedging(new HedgingStrategyOptions<HttpResponseMessage>
{
    MaxHedgedAttempts = 3,
    HedgingDelay = TimeSpan.FromSeconds(3),
    ShouldHandle = new OutcomePredicate<HandleHedgingArguments, HttpResponseMessage>()
        .HandleResult(r => !r.IsSuccessStatusCode)
        .HandleException<HttpRequestException>(),
    HedgingActionGenerator = args =>
    {
        return async ()
        {
             HttpResponseMessage response = await CreateHedgedResponseAsync();
             return response;
        }
    }
});

// Non-generic hedging for handling of multiple result types
builder.AddHedging(new HedgingStrategyOptions
{
    MaxHedgedAttempts = 3,
    HedgingDelay = TimeSpan.FromSeconds(3),
    Handler = new HedgingHandler().SetHedging<HttpResponseMessage>(handler =>
    {
        handler.ShouldHandle
                .HandleResult(r => !r.IsSuccessStatusCode)
                .HandleException<HttpRequestException>();

        handler.HedgingActionGenerator = args =>
        {
            return async ()
            {
                 HttpResponseMessage response = await CreateHedgedResponseAsync();
                 return response;
            }
        };
    })
});

This API is very similar to one introduced for fallback strategy (#1158).

Confirm the following

  • I started this PR by branching from the head of the default branch
  • I have targeted the PR to merge into the default branch
  • I have included unit tests for the issue/feature
  • I have successfully run a local build

@martintmk martintmk added the v8 Issues related to the new version 8 of the Polly library. label Apr 26, 2023
@martintmk martintmk added this to the v8.0.0 milestone Apr 26, 2023
@martintmk martintmk self-assigned this Apr 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2023

Codecov Report

Merging #1160 (8f04b74) into main (6cc8329) will increase coverage by 0.36%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1160      +/-   ##
==========================================
+ Coverage   81.85%   82.21%   +0.36%     
==========================================
  Files         253      267      +14     
  Lines        5780     5898     +118     
  Branches      961      967       +6     
==========================================
+ Hits         4731     4849     +118     
  Misses        844      844              
  Partials      205      205              
Flag Coverage Δ
linux ?
macos 82.21% <100.00%> (+0.36%) ⬆️
windows 82.21% <100.00%> (+0.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Polly.Core/Fallback/FallbackHandler.Handler.cs 100.00% <ø> (ø)
src/Polly.Core/Fallback/FallbackHandler.TResult.cs 100.00% <ø> (ø)
src/Polly.Core/Fallback/FallbackHandler.cs 100.00% <ø> (ø)
src/Polly.Core/Fallback/VoidFallbackHandler.cs 100.00% <ø> (ø)
src/Polly.Core/Hedging/HandleHedgingArguments.cs 100.00% <100.00%> (ø)
...Hedging/HedgingActionGeneratorArguments.TResult.cs 100.00% <100.00%> (ø)
...ly.Core/Hedging/HedgingActionGeneratorArguments.cs 100.00% <100.00%> (ø)
src/Polly.Core/Hedging/HedgingConstants.cs 100.00% <100.00%> (ø)
src/Polly.Core/Hedging/HedgingDelayArguments.cs 100.00% <100.00%> (ø)
src/Polly.Core/Hedging/HedgingHandler.Handler.cs 100.00% <100.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

src/Polly.Core/Hedging/HedgingActionGenerator.TResult.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingActionGenerator.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingHandler.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingHandler.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingResilienceStrategy.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingStrategyOptions.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingStrategyOptions.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/HedgingStrategyOptions.cs Outdated Show resolved Hide resolved
src/Polly.Core/Hedging/OnHedgingArguments.cs Outdated Show resolved Hide resolved
src/Polly.Core/Strategy/NoOutcomeGenerator.cs Outdated Show resolved Hide resolved
Copy link
Member

@martincostello martincostello 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. Could you add it to the benchmarks?

@martintmk
Copy link
Contributor Author

Looks good. Could you add it to the benchmarks?

Yup, once the hedging strategy is implemented.

@martintmk martintmk enabled auto-merge (squash) April 26, 2023 10:48
@martintmk martintmk merged commit 38ae8fd into main Apr 26, 2023
@martintmk martintmk deleted the hedging branch April 26, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 Issues related to the new version 8 of the Polly library.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants