ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.31k stars 1.63k forks source link

#1844 More open customization of Polly use #1845

Closed RaynaldM closed 6 months ago

RaynaldM commented 9 months ago

In certain contexts, we need to be able to fully tune the way Polly is used for timeouts and circuit-breakers, but not only that. With this in mind, I'm proposing in this PR to open up the use of AddPolly by adding :

public static IOcelotBuilder AddPolly<T>(this IOcelotBuilder builder, Dictionary<Type, Func<Exception, Error>> errorMapping)
     where T : class, IPollyQoSProvider<HttpResponseMessage> =>
     AddPolly<T>(builder, GetDelegatingHandler, errorMapping);

 public static IOcelotBuilder AddPolly<T>(this IOcelotBuilder builder, QosDelegatingHandlerDelegate delegatingHandler)
     where T : class, IPollyQoSProvider<HttpResponseMessage> =>
     AddPolly<T>(builder, delegatingHandler, ErrorMapping);

 public static IOcelotBuilder AddPolly<T>(this IOcelotBuilder builder)
     where T : class, IPollyQoSProvider<HttpResponseMessage> =>
     AddPolly<T>(builder, GetDelegatingHandler, ErrorMapping);

Thanks to this, we will be able to use our own implementations of IPollyQoSProvider<HttpResponseMessage>, QosDelegatingHandlerDelegate and Dictionary<Type, Func<Exception, Error>>

raman-m commented 9 months ago

@ggnaegi FYI Easy to review, but it is marked for Dec'23 release...

raman-m commented 9 months ago

@RaynaldM Thanks for the PR! New extensions will be useful. Why not to test them somehow?

raman-m commented 6 months ago

@ggnaegi Please review!

raman-m commented 6 months ago

But the build is still 🔴

raman-m commented 6 months ago

@RaynaldM Is your development finished here?

raman-m commented 6 months ago

🤕

raman-m commented 6 months ago

If you love to press GitHub buttons then I'm letting you to finish your PR. Please fix compilation errors!

raman-m commented 6 months ago

My understanding this PR should be merged first before #1914 ... right?

RaynaldM commented 6 months ago

My understanding this PR should be merged first before #1914 ... right?

Exactly, it is an intermediate step