dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.19k stars 9.93k forks source link

Should Microsoft.Extensions.Http.Polly be deprecated in favour of Microsoft.Extensions.Http.Resilience? #57209

Open IanKemp opened 1 month ago

IanKemp commented 1 month ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

Microsoft.Extensions.Http.Polly references Polly.Extensions.Http, the latter of which is marked as deprecated and tells you to use Microsoft.Extensions.Http.Resilience, so should Microsoft.Extensions.Http.Polly continue to exist in a non-deprecated form? If so, when should one use Microsoft.Extensions.Http.Polly over Microsoft.Extensions.Http.Resilience?

It is also quite confusing for Microsoft.Extensions.Http.Polly to exist as a v8 package when it doesn't actually reference v8 of Polly.

Related: #55356

Describe the solution you'd like

Guidance on usage of Microsoft.Extensions.Http.Polly going forward.

Additional context

No response

martincostello commented 1 month ago

Speaking as one of the maintainers of Polly:

when should one use Microsoft.Extensions.Http.Polly over Microsoft.Extensions.Http.Resilience?

Only for backwards compatibility - you'll get the best performance with Polly v8 and the Polly.Core functionality that Microsoft.Extensions.Http.Resilience is built on top of. This is non-zero effort to do as the APIs are quite different, so the older approach is still fine to use if you can't make that investment.

For new usage of Polly, I would always recommend using Microsoft.Extensions.Http.Resilience.

It is also quite confusing for Microsoft.Extensions.Http.Polly to exist as a v8 package when it doesn't actually reference v8 of Polly.

This is just the coincidence that Polly v8 and ASP.NET Core 8 were released around the same time - the version numbers matching is unintentional. Microsoft.Extensions.Http.Polly is compatible with Polly v8, but we didn't push to get the version here updated as it wouldn't have brought any noticeable improvements and would have been very last-minute before shipping as we needed to release before .NET 8 so that the resilience libraries could depend on a stable version of Polly.

amcasey commented 3 weeks ago

Possibly a question for @RussKie?

RussKie commented 3 weeks ago

I don't think I can add anything to what @martincostello said earlier.

From a consumer standpoint I can totally see the confusion - it took me several minutes to figure out which package belongs to whom as the names are too similar.

If Microsoft.Extensions.Http.Resilience is a strategic replacement for Microsoft.Extensions.Http.Polly, and the latter is only kept for backwards compatibility, then perhaps it's worth considering marking Microsoft.Extensions.Http.Polly as obsolete, updating its readme and providing a guidance on how to migrate to Microsoft.Extensions.Http.Resilience.

@captainsafia @halter73 thoughts?

captainsafia commented 3 weeks ago

If Microsoft.Extensions.Http.Resilience is a strategic replacement for Microsoft.Extensions.Http.Polly, and the latter is only kept for backwards compatibility, then perhaps it's worth considering marking Microsoft.Extensions.Http.Polly as obsolete, updating its readme and providing a guidance on how to migrate to Microsoft.Extensions.Http.Resilience.

As I recall, obsoleting Microsoft.Extensions.Http.Polly was the plan back at the end of .NET 8. We didn't end up pursuing that however because there was still a vestigial reference to APIs in Microsoft.Extensions.Http.Polly from Microsoft.Extensions.Http.Resilience. We decide to hold off on the deprecation until Microsoft.Extensions.Http.Resilience no longer took any dependency on Microsoft.Extensions.Http.Polly.

It looks like Microsoft.Extensions.Htt.Polly was removed as a dependency back in September via https://github.com/dotnet/extensions/pull/4475. Based on that, I see no reason not to move forward with obsoletion.