Add the new overloads that accept lambda that configures the options

[martintmk]

Details on the issue fix or feature implementation

Just creating this PR to discuss whether adding new extensions that accept lambdas that configure the options is something we want to have built-in. It simplifies the configuration of predicates, events and generators.

Adding retry strategy using the options:

builder.AddRetry(new RetryStrategyOptions
{
    RetryCount = 4,
    BackoffType = RetryBackoffType.Exponential,
    ShouldRetry = new OutcomePredicate<ShouldRetryArguments>().HandleResult(-1).HandleException<InvalidOperationException>(),
    OnRetry = new OutcomeEvent<OnRetryArguments>().Register(()=> Console.WriteLine("Retry occurred!")),
});

Using the configure callback:

builder.AddRetry(o =>
{
    o.RetryCount = 4;
    o.BackoffType = RetryBackoffType.Exponential;
    o.ShouldRetry.HandleResult(-1).HandleException<InvalidOperationException>();
    o.OnRetry.Register(() => Console.WriteLine("Retry occurred!"));
});

Note that the overload that accepts the options directly needs to stay as it will be API that can integrate with IOptions.

Is the second API (in addition to the first one) something we want to have built-in?

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

[martincostello]

Seems a reasonable approach to me. I would imagine that it should compose well with DI so that people could load configuration settings from configuration?

For example:

builder.AddRetry(o =>
{
    o.RetryCount = configuration.Get<int>("MyRetryCount");
});

[juraj-blazek]

It's one more overload for each strategy, but I think it makes the configuration of the predicates/events/generators much more readable

[martintmk]

Seems a reasonable approach to me. I would imagine that it should compose well with DI so that people could load configuration settings from configuration?

For example:

builder.AddRetry(o =>
{
    o.RetryCount = configuration.Get<int>("MyRetryCount");
});

For DI scenario I would recommend this approach (still does not negate the approach above):

return new ServiceCollection()
    .Configure<RetryStrategyOptions>(options =>
    {
      o.RetryCount = 4;
      o.BackoffType = RetryBackoffType.Exponential;
      o.ShouldRetry.HandleResult(-1).HandleException<InvalidOperationException>();
      o.OnRetry.Register(() => Console.WriteLine("Retry occurred!"));
    })
    .AddResilienceStrategy("my-retry", context =>
    {
        // resolve options from DI
        var options = context.ServiceProvider.GetRequiredService<IOptions<RetryStrategyOptions>>().Value;
        context.Builder.AddRetry(options);
    })

You can just resolve complete configured options from DI.

[martincostello]

OK in my use case then I'd probably not use the new overload and do it like your example, as I would want some values to be in external configuration (e.g. JSON files etc.) not hard-coded.

return new ServiceCollection()
    .Configure<RetryStrategyOptions, IConfiguration>((o, configuration) =>
    {
      o.RetryCount = configuration.Get<int>("MyRetryCount");
      o.BackoffType = RetryBackoffType.Exponential;
      o.ShouldRetry.HandleResult(-1).HandleException<InvalidOperationException>();
      o.OnRetry.Register(() => Console.WriteLine("Retry occurred!"));
    })
    .AddResilienceStrategy("my-retry", context =>
    {
        // resolve options from DI
        var options = context.ServiceProvider.GetRequiredService<IOptions<RetryStrategyOptions>>().Value;
        context.Builder.AddRetry(options);
    })

[martintmk]

OK in my use case then I'd probably not use the new overload and do it like your example, as I would want some values to be in external configuration (e.g. JSON files etc.) not hard-coded.

return new ServiceCollection()
    .Configure<RetryStrategyOptions, IConfiguration>((o, configuration) =>
    {
      o.RetryCount = configuration.Get<int>("MyRetryCount");
      o.BackoffType = RetryBackoffType.Exponential;
      o.ShouldRetry.HandleResult(-1).HandleException<InvalidOperationException>();
      o.OnRetry.Register(() => Console.WriteLine("Retry occurred!"));
    })
    .AddResilienceStrategy("my-retry", context =>
    {
        // resolve options from DI
        var options = context.ServiceProvider.GetRequiredService<IOptions<RetryStrategyOptions>>().Value;
        context.Builder.AddRetry(options);
    })

You can also use this right?

return new ServiceCollection()
    .Configure<RetryStrategyOptions>(configuration) // this binds values from configuration
    .Configure<RetryStrategyOptions>(o =>
    {
      // just configure the callbacks here
      o.ShouldRetry.HandleResult(-1).HandleException<InvalidOperationException>();
      o.OnRetry.Register(() => Console.WriteLine("Retry occurred!"));
    })
    .AddResilienceStrategy("my-retry", context =>
    {
        // resolve options from DI
        var options = context.ServiceProvider.GetRequiredService<IOptions<RetryStrategyOptions>>().Value;
        context.Builder.AddRetry(options);
    })

[martincostello]

In my simple example yes, but I'd probably compose it up with a more structured configuration file based on other things relating to different downstream APIs etc, at which point there'd be different settings for different named strategies so it would end up getting into named options so I'd probably be binding things explicitly from options that are already bound onto the configuration.

[martintmk]

Won't be required after #1200.