-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Change ConcurrenyMIddleware service extensions method #19324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change ConcurrenyMIddleware service extensions method #19324
Conversation
Breaking change procedures require keeping the old extensions and adding new ones. |
As @Tratcher said, this is an unfortunate situation since it really shouldn't have been done this way but now that it was, it's a breaking change to fix it :). I expect what we would prefer to do is this:
But we'd also need to leave the existing methods in place for 5.0 at least (marked as obsolete) and then consider removing them later. |
src/Middleware/ConcurrencyLimiter/src/ConcurrencyMiddlewareServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
There is an open issue (#19823) on adding more feature to Concurency Limiter. Should I close this PR to prevent future breaking changes since the naming can be handled in the issue? |
src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterBuilderExtensions.cs
Show resolved
Hide resolved
src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterBuilderExtensions.cs
Show resolved
Hide resolved
@Kahbazi Sorry for the slow feedback on this, but the short answer is yes, we've decided to close this. This PR clearly improves on what we currently have in our opinion, but we think we are going to want to make further configuration changes to support endpoint-based concurrency limiting (#20835) with named policies similar to how CorsOptions works today. It seems unlikely that endpoint-based concurrency will make it in time for .NET 5, but even so we don't want to make a breaking change in one major release just to break the same API in the next. I plan to file an issue with the new proposed design soon. |
The design we want is very similar to what you outlined in #19823 (comment). I'm looking at that as the starting point for the new proposal. I'll probably just respond to that issue. |
Change ConcurrenyMIddleware service extensions method based on #12453 (comment)