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

Demonstrate how to create dynamic strategies with complex keys #1366

Merged
merged 13 commits into from
Jul 14, 2023

Conversation

martintmk
Copy link
Contributor

@martintmk martintmk commented Jun 29, 2023

Details on the issue fix or feature implementation

This sample demonstrates how to define a HTTP resilience strategy that is created and configured for different combinations of endpoints/resources.

The idea is to cache the strategy by complex EndpointKey. This sample also demonstrates how EndpointKey can be used to create dynamic strategies where some inner strategies are conditional.

Paired with the EndpointKey is the configuration of ResilienceStrategyRegistryOptions<EndpointKey> where we customize how the registry creates and caches the strategies.

@martincostello

Contributes to #1365

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 Jun 29, 2023
@martintmk martintmk added this to the v8.0.0 milestone Jun 29, 2023
@martintmk martintmk mentioned this pull request Jun 29, 2023
@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Merging #1366 (1275507) into main (8bb251a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1366   +/-   ##
=======================================
  Coverage   83.80%   83.80%           
=======================================
  Files         273      273           
  Lines        6476     6476           
  Branches     1010     1010           
=======================================
  Hits         5427     5427           
  Misses        840      840           
  Partials      209      209           
Flag Coverage Δ
linux ?
macos 83.80% <ø> (ø)
windows 83.80% <ø> (?)

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

@martincostello
Copy link
Member

Thanks for this - I'll have a play around with it now.

In my case, these options objects are bound from IConfiguration via DI, so they observe configuration changes. In this way of doing things, how would they observe changes? With the code I already have, we explicitly just clear the policy registry when the configuration changes and then policies are then rebuilt from scratch to account for any configuration change.

@martincostello
Copy link
Member

Off the top of my head, would nulling out the ResilienceStrategyProvider<T> and then just recreating it from the service provider as a transient solve that?

@martincostello
Copy link
Member

It's also getting a bit bootstrap-y, as I need the options from configuration to configure the service container, as that's where the endpoint options come from.

@martincostello
Copy link
Member

This is starting to get a bit complicated - tomorrow I'll try and extract the relevant parts of the v7 Polly code into a runnable-sample with the existing tests - then it'll be easier to see what the migration path would be and if there's anything we can do to make that easier.

@martintmk
Copy link
Contributor Author

@martincostello

In my case, these options objects are bound from IConfiguration via DI, so they observe configuration changes. In this way of doing things, how would they observe changes?

In case you configure the EndpointOptions in DI the sample can be simplified. Instead of including it in the key you can just resolve those from DI and use it to configure resilience strategy. Additionally, you can instruct Polly to dynamically reload the resilience strategies when these options are changed.

Instead of:

var endpointOptions = context.StrategyKey.EndpointOptions;

You do:

// resolve the options from DI
var endpointOptions = context.GetOptions<EndpointOptions>(context.StrategyKey.EndpointName);

// enable dynamic reload of the resilience strategy being configured whenever the named options are changed
context.EnableReloads<EndpointOptions>(context.StrategyKey.EndpointName);

@martintmk martintmk force-pushed the mtomka/demonstrate-api-usage branch from 2a4650b to 66af7d6 Compare July 10, 2023 10:59
@martintmk
Copy link
Contributor Author

@martincostello is it OK to merge this one or we are waiting for alpha.6?

@martincostello
Copy link
Member

Sure - btw have you forgotten to re-open Slack since your vacation? 😄

@martintmk
Copy link
Contributor Author

Sure - btw have you forgotten to re-open Slack since your vacation? 😄

Ooops, I check it maybe once, twice a day so the risk of me missing something is high :D

@martintmk martintmk enabled auto-merge (squash) July 14, 2023 12:49
@martintmk martintmk merged commit 572558a into main Jul 14, 2023
16 checks passed
@martintmk martintmk deleted the mtomka/demonstrate-api-usage branch July 14, 2023 14:08
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.

2 participants