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

Use Polly v8 syntax #1875

Closed
RaynaldM opened this issue Jan 5, 2024 · 4 comments · Fixed by #1907 or #1914
Closed

Use Polly v8 syntax #1875

RaynaldM opened this issue Jan 5, 2024 · 4 comments · Fixed by #1907 or #1914
Assignees
Labels
feature A new feature Feb'24 February 2024 release merged Issue has been merged to dev and is waiting for the next release QoS Ocelot feature: Quality of Service aka Polly
Milestone

Comments

@RaynaldM
Copy link
Collaborator

RaynaldM commented Jan 5, 2024

Use Polly 8's specific syntax to define resiliencePipelines (vs. PollyPolicies)

Polly v8 introduces the concept of resilience pipelines, a powerful tool that blends one or more resilience strategies. This new API foundation echoes the Policy Wrap of previous versions but is now integrated seamlessly into the core API. All strategies start with a pipeline.

in v7, we written

// Create and use the IAsyncPolicy
IAsyncPolicy asyncPolicy = Policy
    .Handle<Exception>()
    .WaitAndRetryAsync(3, _ => TimeSpan.FromSeconds(1));

in v8, it becomes

// Use the ResiliencePipelineBuilder to start building the resilience pipeline
ResiliencePipeline pipeline = new ResiliencePipelineBuilder()
    .AddRetry(new RetryStrategyOptions
    {
        ShouldHandle = new PredicateBuilder().Handle<Exception>(),
        Delay = TimeSpan.FromSeconds(1),
        MaxRetryAttempts = 3,
        BackoffType = DelayBackoffType.Constant
    })
    .Build(); // After all necessary strategies are added, call Build() to create the pipeline.
@RaynaldM RaynaldM self-assigned this Jan 5, 2024
@RaynaldM RaynaldM added feature A new feature medium effort Likely a few days of development effort in progress Someone is working on the issue. Could be someone on the team or off. QoS Ocelot feature: Quality of Service aka Polly Jan'24 January 2024 release labels Jan 5, 2024
@raman-m
Copy link
Member

raman-m commented Jan 5, 2024

Wow! Good to know about new Polly 8 features... I see you are big fan of Polly 😉

@raman-m
Copy link
Member

raman-m commented Jan 5, 2024

You need to choose release number: Annual (aka 24.0) or Jan'24 (aka 24.1)
We can include this upgrade even in Annual'23: I see the feature branch, seems you're going to create a PR soon...
Finally you need to choose release number to include this feature in. OK?

@RaynaldM
Copy link
Collaborator Author

RaynaldM commented Jan 5, 2024

In 24.1

@raman-m raman-m linked a pull request Jan 8, 2024 that will close this issue
@raman-m raman-m added this to the January'24 milestone Jan 8, 2024
@RaynaldM RaynaldM linked a pull request Jan 9, 2024 that will close this issue
@raman-m
Copy link
Member

raman-m commented Feb 17, 2024

@RaynaldM commented on Jan 5:

In 24.1

No. It will belong to version 23.1 actually. See the milestone version and our Slack channels!

@raman-m raman-m added Feb'24 February 2024 release and removed in progress Someone is working on the issue. Could be someone on the team or off. Jan'24 January 2024 release labels Mar 1, 2024
@raman-m raman-m modified the milestones: January'24, February'24 Mar 1, 2024
RaynaldM added a commit that referenced this issue Mar 15, 2024
* #1844 More open customization of Polly use

* First step: implementation done, The UTs have yet to be determined

* reverted changes in PollyPoliciesDelegatingHandlerTests
created tests for  PollyResiliencePipelineDelegatingHandler
created tests for PollyQoSResiliencePipelineProvider

* renaming variables for better clarity in PollyQoSResiliencePipelineProviderTests

* The most significant changes involve updates to the `OcelotBuilderExtensions` and `PollyQoSResiliencePipelineProvider` classes, as well as modifications to the `PollyQoSTests` class.

In the `OcelotBuilderExtensions` class, the `GetDelegatingHandler` method is now used instead of `GetDelegatingHandlerV7` in the `AddPolly` methods. The `AddPolly` method without parameters has been simplified using a lambda expression. The `GetDelegatingHandler` method now returns a new instance of `PollyResiliencePipelineDelegatingHandler` instead of `PollyPoliciesDelegatingHandler`.

In the `PollyQoSResiliencePipelineProvider` class, a comment has been added suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline` due to its thread-safe nature. The `MinimumThroughput` property in `circuitBreakerStrategyOptions` now uses the value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.

In the `PollyQoSTests` class, the `QoSOptions` used in the test configurations have been updated. Two tests, `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route`, have been modified to repeat the same request as the minimum `ExceptionsAllowedBeforeBreaking` is now 2. The `GivenThereIsAPossiblyBrokenServiceRunningOn` method now delays for 2.1 seconds when the request count is 2, to ensure the circuit is open.

Changes:

1. `OcelotBuilderExtensions` class updated to use `GetDelegatingHandler` method.
2. `AddPolly` methods in `OcelotBuilderExtensions` class simplified.
3. `GetDelegatingHandler` method in `OcelotBuilderExtensions` class now returns a new instance of `PollyResiliencePipelineDelegatingHandler`.
4. Comment added in `PollyQoSResiliencePipelineProvider` class suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline`.
5. `MinimumThroughput` property in `circuitBreakerStrategyOptions` updated to use value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.
6. `QoSOptions` in `PollyQoSTests` class updated.
7. `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route` tests in `PollyQoSTests` class updated.
8. `GivenThereIsAPossiblyBrokenServiceRunningOn` method in `PollyQoSTests` class updated to delay for 2.1 seconds when request count is 2.

* end of implementation

* remove "Microsoft.VisualStudio.Azure.Containers.Tools.Targets" packages in sample

* Use `ResiliencePipelineRegistry` in `PollyQoSResiliencePipelineProvider`. This new implementation replaces the previous dictionary-based approach for managing cache for resilience pipelines.

* fix conflict

* end of merge of develop (+polly package uptdate)

* fix polly update

* #1844 More open customization of Polly use

* First step: implementation done, The UTs have yet to be determined

* reverted changes in PollyPoliciesDelegatingHandlerTests
created tests for  PollyResiliencePipelineDelegatingHandler
created tests for PollyQoSResiliencePipelineProvider

* renaming variables for better clarity in PollyQoSResiliencePipelineProviderTests

* The most significant changes involve updates to the `OcelotBuilderExtensions` and `PollyQoSResiliencePipelineProvider` classes, as well as modifications to the `PollyQoSTests` class.

In the `OcelotBuilderExtensions` class, the `GetDelegatingHandler` method is now used instead of `GetDelegatingHandlerV7` in the `AddPolly` methods. The `AddPolly` method without parameters has been simplified using a lambda expression. The `GetDelegatingHandler` method now returns a new instance of `PollyResiliencePipelineDelegatingHandler` instead of `PollyPoliciesDelegatingHandler`.

In the `PollyQoSResiliencePipelineProvider` class, a comment has been added suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline` due to its thread-safe nature. The `MinimumThroughput` property in `circuitBreakerStrategyOptions` now uses the value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.

In the `PollyQoSTests` class, the `QoSOptions` used in the test configurations have been updated. Two tests, `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route`, have been modified to repeat the same request as the minimum `ExceptionsAllowedBeforeBreaking` is now 2. The `GivenThereIsAPossiblyBrokenServiceRunningOn` method now delays for 2.1 seconds when the request count is 2, to ensure the circuit is open.

Changes:

1. `OcelotBuilderExtensions` class updated to use `GetDelegatingHandler` method.
2. `AddPolly` methods in `OcelotBuilderExtensions` class simplified.
3. `GetDelegatingHandler` method in `OcelotBuilderExtensions` class now returns a new instance of `PollyResiliencePipelineDelegatingHandler`.
4. Comment added in `PollyQoSResiliencePipelineProvider` class suggesting the use of `ResiliencePipelineRegistry<TKey>.GetOrAddPipeline`.
5. `MinimumThroughput` property in `circuitBreakerStrategyOptions` updated to use value from `route.QosOptions.ExceptionsAllowedBeforeBreaking`.
6. `QoSOptions` in `PollyQoSTests` class updated.
7. `Should_open_circuit_breaker_then_close` and `Open_circuit_should_not_effect_different_route` tests in `PollyQoSTests` class updated.
8. `GivenThereIsAPossiblyBrokenServiceRunningOn` method in `PollyQoSTests` class updated to delay for 2.1 seconds when request count is 2.

* Use `ResiliencePipelineRegistry` in `PollyQoSResiliencePipelineProvider`. This new implementation replaces the previous dictionary-based approach for managing cache for resilience pipelines.

* end of implementation

* remove "Microsoft.VisualStudio.Azure.Containers.Tools.Targets" packages in sample

* fix conflict

* end of merge of develop (+polly package uptdate)

* fix polly update

* renaming

* fix compile error

* The most significant changes involve the modification of the `errorMapping` type from `Dictionary<Type, Func<Exception, Error>>` to `IDictionary<Type, Func<Exception, Error>>` across multiple files. This change is aimed at enhancing code flexibility and testability. Additionally, the `AddPolly<T>` and `AddPollyV7<T>` methods in `OcelotBuilderExtensions.cs` have been updated to accept `IDictionary` instead of `Dictionary`.

Here are the changes in detail:

1. The type of `errorMapping` was changed from `Dictionary<Type, Func<Exception, Error>>` to `IDictionary<Type, Func<Exception, Error>>` in several places across multiple files to make the code more flexible and easier to test.
2. The `AddPolly<T>` and `AddPollyV7<T>` methods in `OcelotBuilderExtensions.cs` were updated to accept `IDictionary` instead of `Dictionary`.
3. The `_mappers` field and the corresponding assignment in the constructor in `HttpExceptionToErrorMapper.cs` were updated to use `IDictionary`.
4. The `WithExceptionsAllowedBeforeBreaking` and `WithTimeoutValue` values in `OcelotBuilderExtensionsTests.cs` and `PollyQoSResiliencePipelineProviderTests.cs` were updated.
5. The `errorMapping` variable in `HttpExceptionToErrorMapperTests.cs` was updated to use `IDictionary`.

* try to fix UTs on CircleCI

* try another settings

* one more test

* post code reveiew and develop merge

* Quick code review by @raman-m

* Re-add lost XML docs from commit 0404c24
Complete docs

* Update Polly version

* Update Polly to 8.3.1

* Code review by @raman-m

* Arrange file layout in a separate directory (V7)
Modify Obsolete attribute texts
Add note in doc

* Update qualityofservice.rst

Update docs according to the Polly v8 implementations and QoS design upgrade

* More XML docs

* Make the base class abstract

* some code cleanup

* some comments

* remove comments

* Re-use logger and fix warning

* Final review of QoS docs

---------

Co-authored-by: Ray <[email protected]>
Co-authored-by: Tomislav Prebeg <[email protected]>
Co-authored-by: Raman Maksimchuk <[email protected]>
Co-authored-by: Guillaume Gnaegi <[email protected]>
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release and removed medium effort Likely a few days of development effort labels Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature Feb'24 February 2024 release merged Issue has been merged to dev and is waiting for the next release QoS Ocelot feature: Quality of Service aka Polly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants