Closed eerhardt closed 7 months ago
Seems reasonable to me, plus would make the API easier to use even without wanting to use AoT.
This seems to be actually a bug, at least in the documentation. The options
parameter in the ResiliencePipelineBuilderExtensions.AddStrategy
methods is described like this:
The options associated with the strategy. If none are provided the default instance of ResilienceStrategyOptions is created.
So one would expect the parameter already to be optional. There is the ´EmptyOptions´ class, but it's internal, so I need to copy it if I don't need any options for my strategy.
This seems to be actually a bug, at least in the documentation. The
options
parameter in theResiliencePipelineBuilderExtensions.AddStrategy
methods is described like this:The options associated with the strategy. If none are provided the default instance of ResilienceStrategyOptions is created.
So one would expect the parameter already to be optional. There is the ´EmptyOptions´ class, but it's internal, so I need to copy it if I don't need any options for my strategy.
These are incorrect docs, thanks for noticing. Both new API and fixed docs are addressed in #2068.
cc @eerhardt
Is your feature request related to a specific problem? Or an existing feature?
The
AddStrategy
extension methods take aResilienceStrategyOptions options
argument, which gets validated using DataAnnotations. This makes the method incompatible with trimming and native AOT, and it is annotated as such:https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs#L68-L70
https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs#L90-L93
https://github.com/App-vNext/Polly/blob/82474bf10b5097d4bf6934100c9daa1efdb02f4b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs#L114-L117
Callers who want to use this API in a trim compatible way need to define an empty class derived from
ResilienceStrategyOptions
and suppress the warning from calling this API.Describe the solution you'd like
We should add overloads to these
AddStrategy
methods that are trim compatible, and don't take anoptions
argument. Similar to theAddPipeline
methods.Additional context
This is being used by Microsoft.Extensions.Http.Resilience here: https://github.com/dotnet/extensions/pull/4962#discussion_r1497965809
Adding this API would make the caller simpler. It wouldn't need to declare an empty class, and suppress the trim warning.