Update AddTransientHttpErrorPolicy once Polly v7 is released

[johnknoop]

I'm not sure if you are already aware of this, but Polly v7 will distinguish between RetryPolicy and AsyncRetryPolicy. Since all methods on the HttpClient are async, it would make sense if AddTransientHttpErrorPolicy only accepted AsyncPolicy (note: not IAsyncPolicy since the sync version of RetryPolicy also implements that interface for some weird reason).

https://github.com/App-vNext/Polly/wiki/Polly-v7-breaking-changes

[reisenberger]

No changes to AddTransientHttpErrorPolicy(...) will be necessary to take advantage of Polly v7 cleaning up the sync/async split in Polly. HttpClientFactory already only accepts IAsyncPolicy<HttpResponseMessage> (see PollyHttpClientBuilderExtensions), which is the right thing to accept.

it would make sense if AddTransientHttpErrorPolicy only accepted AsyncPolicy (note: not IAsyncPolicy since the sync version of RetryPolicy also implements that interface)

That does not hold for Polly v7. Polly v7 clears up the weak sync/async split in Polly (inherited from previous maintainers but now being removed). For clarity again: No changes to AddTransientHttpErrorPolicy(...) will be necessary to take advantage of the changed sync/async handling in Polly v7.

---

The only relevant change that could be made on the Microsoft side would be (if desired, at a future release) to change the minimum supported version of Polly, in HttpClientFactory, to Polly v7.0.0. This would prevent users configuring eg .AddTransientHttpErrorPolicy(x => x.Retry(3)) (with v7 this will simply not compile) when .AddTransientHttpErrorPolicy(x => x.RetryAsync(3)) should be used.

The only other breaking change in Polly v7.0.0 is changes to cache provider interfaces. For cache providers which the Polly team maintains (Polly.Caching.Memory, Polly.Caching.Distributed and Polly.Caching.Serialization.Json), we will simultaneously release upgraded cache providers. Thus, if Microsoft chooses to update HttpClientFactory in a future release to specify Polly v7.0.0 as the minimum-supported version, the only users forced to adjust their code for a breaking change would be any that had implemented their own custom cache providers for Polly CachePolicy.

Thanks @johnknoop for raising this 👍 ; a good prompt to document here how HttpClientFactory could be updated for Polly v7.

[glennc]

Thanks @reisenberger I appreciate all the context. I've fallen out of touch with your v7 work. Do you know when you plan to ship it?

[johnknoop]

Thanks for the clarification

[reisenberger]

@glennc We are planning to ship Polly v7 in the next fortnight: I will advise when it ships. Polly v7 is both a maintenance update:

• Clarify separation of sync and async policies - no impact on HttpClientFactory except improved usability for users - could update HttpClientFactory to Polly >= v7.0 as discussed above
• Enable collection initialization syntax for PolicyRegistry - simplifies a few StartUp configuration scenarios for HttpClientFactory with PolicyRegistry
• Enable cache policies to cache default(TResult) - bug fix

And major new features:

• Enable extensibility by custom policies - IE allow users to author their own Polly-compatible policies, turning Polly into user-extensible middleware for any outbound call - (the equivalent of what HttpClient has via custom DelegatingHandlers; but for any Polly-wrapped execution, not just HttpClient)
• the launch of Simmy, a software-level fault-injection and chaos-injection tool based on custom Polly policies. Inject faults in a controllable manner into any calls you already guard with Polly. Including via HttpClient and HttpClientFactory. (May launch a few weeks behind Polly v7, but already build-able from the github repo)

Thanks!

/cc @joelhulen

[reisenberger]

@glennc Polly v7 is shipped. If you wish to upgrade HttpClientFactory for .NET Core 3.0 to reference Polly v7, we recommend that HttpClientFactory reference Polly v7.0.3 v7.1.0 as the minimum version.

As described in more detail above:

Advantages: Improved async syntax enforcement.
Risk of breaking change: Only for a user who has implemented their own custom cache provider.

Let me know if you want to proceed. HttpClientFactory also references to Polly.Extensions.Http, so if we go ahead, the sequence would be:

• Polly team publish a Polly.Extensions.Http v3.0.0, which also references min Polly v7.0.3 v7.1.0 (... to avoid any diamond dependency conflicts)
• HttpClientFactory is (in a single PR) repointed to reference Polly v7.0.3 v7.1.0 and Polly.Extensions.Http v3.0.0

Thanks

/cc @joelhulen

[rynowak]

I'll catch up with @glennc and discuss with him. I can't think of a reason why we wouldn't do this. We want users to get the best version of things.

[reisenberger]

Pinging an update: Note that we are planning to publish a Polly v7.1.0 (intended by end of week) which would be the version to reference, if we go for this.

Will update, soon as that ships.

[rynowak]

Great!

[altso]

Any plans to upgrade HttpClientFactory v2.x to Polly 7?

[rynowak]

Yes, we're currently planning that as part of 3.0

[altso]

@rynowak apologies for the confusion. I understand that Microsoft.Extensions.Http.Polly 3.0.0 will have a dependency on Polly 7.1.0 (or higher). Will you be also releasing Microsoft.Extensions.Http.Polly 2.x.x (e.g. 2.3.0) with a dependency on Polly 7.1.0?

[rynowak]

No, we don't change dependency versions in a patch release.

[altso]

Got it. Thanks for the clarification.

[reisenberger]

@rynowak @glennc Polly v7.1.0 is now on nuget.

Here the procedure to update inter-related components all to reference Polly v7.1.0. Please let me know when you want me to enact this?

Thanks!

[rynowak]

@reisenberger we are ready to do this whenever. Ideally we could get it done in the next two weeks for the next preview release.

[reisenberger]

@rynowak Thanks! I am in a conference all week but should get to this at the weekend.

[rynowak]

great!

[rynowak]

Here's a draft PR. #1313

I don't expect it to pass now since we're waiting for the new package.

[rynowak]

This is done!