search

App-vNext / Polly

Polly is a .NET resilience and transient-fault-handling library that allows developers to express policies such as Retry, Circuit Breaker, Timeout, Bulkhead Isolation, and Fallback in a fluent and thread-safe manner. From version 6.0.1, Polly targets .NET Standard 1.1 and 2.0+.
https://www.thepollyproject.org
BSD 3-Clause "New" or "Revised" License
13.33k stars 1.22k forks source link

[Feature request]: Add TimeProvider argument to resilience extension methods #1983

Closed silkfire closed 7 months ago

silkfire commented 7 months ago

Is your feature request related to a specific problem? Or an existing feature?

When creating a ResiliencePipelineBuilder, the consumer is offered to set a custom ITimeProvider by setting the TimeProvider property on the builder. This is not possible when using e.g. HTTP resilience extension methods like AddStandardResilienceHandler as the builder is created internally.

Describe the solution you'd like

It'd be useful if an overload was provided that allows the consumer of the extension methods to specify a ITimeProvider argument of their own choice, so as to increase flexibility for e.g. unit tests.

Additional context

No response

martincostello commented 7 months ago

Could you clarify exactly which methods you want this added to?

If it's the AddStandardResilienceHandler() methods, they don't live in this repository but in dotnet/extensions.

silkfire commented 7 months ago

The methods I was referring to were AddStandardResiliencePipeline and AddResiliencePipeline. I wasn't actually aware that they belonged to a different repository. Should I open an issue there instead?

martincostello commented 7 months ago

I don't seem to be able to find AddStandardResiliencePipeline() in either repository with GitHub code search. AddStandardResilienceHandler() is in dotnt/extensions, where as AddResiliencePipeline() lives here.

https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs#L101

martintmk commented 7 months ago

@silkfire If TimeProvider is registered in the DI, the AddResiliencePipeline will automatically use and assign it:

https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs#L282

silkfire commented 7 months ago

I don't seem to be able to find AddStandardResiliencePipeline() in either repository with GitHub code search. AddStandardResilienceHandler() is in dotnt/extensions, where as AddResiliencePipeline() lives here.

https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs#L101

Apologies, it was late and I got the methods mixed up. The one I intended was AddStandardResilienceHandler().

@silkfire If TimeProvider is registered in the DI, the AddResiliencePipeline will automatically use and assign it:

https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs#L282

Interesting, I wasn't aware of that, this might actually be sufficient for my use case. Thank you.