resilience-strategy-with-polly icon indicating copy to clipboard operation
resilience-strategy-with-polly copied to clipboard

Nuget me up!

Open Jetski5822 opened this issue 5 years ago • 7 comments

This package is awesome - some really goo ideas, fancy throwing it up on to nuget?

Jetski5822 avatar Apr 11 '19 08:04 Jetski5822

@Jetski5822 thanks appreciate it!

Regarding the nuget, maybe this might be a new project for Polly.Contrib. We might have several nuget packages to handle different resilience strategies in a generic way, such as Azure SQL Databases, Azure Service Bus, etc. Also, we would need to make them extensible in order to people can inject additionals strategies to the builder (and a lot of enhancements as well, I didn't write this guy thinking about a nuget tho 😅 ), etc what do you think @reisenberger?

vany0114 avatar Apr 12 '19 00:04 vany0114

@vany0114 I'll dig in and have a look!

reisenberger avatar Apr 13 '19 07:04 reisenberger

Hi @vany0114 . It is great to see everything you are doing around Polly! I dug into the code (and blog post), but quickly, so, please, feel free to say if I have misunderstood anything.

It looks broadly as if this project does three different kinds of thing around Polly:

(1) Specific helper methods around SQL errors (and suggestion we could do similar for eg Azure Service Bus; other services). (2) New builder syntax for the policies / policywrap (3) some combining of the sync and async configuration syntax.


If we wanted to take some of this into a/ Polly-Contrib project/s, it might be better (and cleaner for users to understand) to tackle some of these ^ concerns separately:

(1) Specific helper methods around SQL errors (or similar for other downstream components)

These could be great additions to Polly-Contrib. A similar concept to Polly.Extensions.Http, which contains "off-the-shelf" configuration for handling standard http-call errors. Polly.Extensions.Sql or Polly.Extensions.AzureServiceBus (etc) could make great contrib projects. They might (like Polly.Extensions.Http) be limited just to the Handle-clause parts of the syntax, unless I have missed some other angle.

(2) New builder syntax for the policies

I like the new builder syntax, but I have different goals / think we have bigger challenges for a syntax overhaul for Polly. The biggest challenge I see is the proliferation of overloads - for example the large number of Retry policy overload options - the fact that there are overloads for many different possible combinations of parameters. This design (which I inherited on the project) is preventing Polly growth. I would like to change the Polly policy configuration syntax (major breaking change!) to something like Policy.Handle<>(/* etc */).Retry(Action<RetryConfiguration>) ... in other words, similar to much of the .NET Core configuration syntax. This Action<RetryConfiguration> pattern is much more open for extension.

I see this ^ as the most pressing syntax problem to solve for Polly.

(3) some combining of the sync and async configuration syntax

Despite looking hard at whether we could merge sync/async policies, the fact that users want and need async delegate hooks for async policies (and if we don't supply this, it leaves users to fall into the pit of failure with those nasty async void problems) ... means probably we will keep the sync/async split in Polly.

(Aside: If an approach such as .Retry(Action<RetryConfiguration>) discussed above was adopted, it might be possible to achieve some commonality of code by having SyncRetryConfiguration : RetryConfiguration and also AsyncRetryConfiguration : RetryConfiguration ....)

(I might have misunderstood what you have done around sync/async tho.)


Please don't be discouraged if you want to nuget this; I am just trying to answer the question which you and @Jetski5822 raised 🙂 , and hopefully provide some useful input! No obligation also to follow any of these suggestions ...

Thanks.

reisenberger avatar Apr 16 '19 21:04 reisenberger

First things first, @reisenberger thanks for taking the time to dig into the code/blog post I really appreciate it!

Well, the idea behind this was to have a common way to build and manage different resilience strategies thru Polly. I needed to handle Azure SQL databases errors, and most of the times I was using the same combinations of policies (that's why the builder has a method called WithDefaultPolicies). So, the main idea was to build specialize resilience strategies using Polly since every strategy will depend on the component you want to handle, it's not the same, a resilient strategy for Azure SQL databases than for Azure Service Bus.

Regarding the syntax, I just thought in a friendly, flexible, intuitive and easy way to use and build the strategy (like Polly does) but as I said earlier, I wasn't thinking to make a library or something like that, I was just encouraging people to think in that way then they could make their specialized strategies given their use cases, so I just purposed a cool way to do that thru a builder.

Regarding combining of the sync and async configuration syntax, actually, I have two separate implementations for both, sync and async policies that I use thru UseAsyncExecutor or UseSyncExecutor, like this:

var resilientAsyncStrategy = builder
    .UseAsyncExecutor()
    .WithDefaultPolicies()
    .Build();

But it really doesn't matter, as I said, that syntax was only an example of one approach I wanted to show.

Wrapping up, the main idea here is that would be nice having specialized (and common) resilience strategies for a different kind of components, such as Azure SQL databases, Azure Service Bus, etc like you did with Polly.Extensions.Http. I really like that idea because it's just matter of download the nuget and set it up and that's it, you don't have to build that strategy over and over again. Having said that I'd really like to do something like you said earlier:

Polly.Extensions.Sql or Polly.Extensions.AzureServiceBus (etc) could make great contrib projects. They might (like Polly.Extensions.Http) be limited just to the Handle-clause parts of the syntax.

^ that's the main idea behind this.

So, please, let me know your thoughts and if you feel we can make this I'll be more than happy to do it after we release Simmy!

Thanks again.

vany0114 avatar Apr 17 '19 02:04 vany0114

Thanks @vany0114 . It sounds like we have a similar perspective. Also, I should be clear: this project is a great example of how people can build their own thing around Polly to suit their needs! And perfect to put all in one project for that. When I was saying "to tackle these concerns separately", that was only in the context of @Jetski5822 's question "Should we nuget this up?" (and perhaps in Polly-Contrib). For nuget release in Polly-Contrib, I would tackle the aspects separately.

Thanks again for all the awesome things you are doing around Polly! 👍 🙏

reisenberger avatar Apr 17 '19 17:04 reisenberger

So @reisenberger. let me know if you'd like to go further with these ideas on Polly-Contrib 😃

vany0114 avatar Apr 17 '19 18:04 vany0114

I currently pulled the whole project in locally, and its working great. Really loving it, if we could get this in to polly contrib is some manner, that would be great!

Jetski5822 avatar Apr 17 '19 18:04 Jetski5822