Skip to content

Obsolete concurrency limiter middleware. - #47571

Merged
mitchdenny merged 6 commits into
dotnet:mainfrom
mitchdenny:obsolete-concurrency-limiter-package
Apr 6, 2023
Merged

Obsolete concurrency limiter middleware.#47571
mitchdenny merged 6 commits into
dotnet:mainfrom
mitchdenny:obsolete-concurrency-limiter-package

Conversation

@mitchdenny

Copy link
Copy Markdown
Member

Addresses #47532

@ghost ghost added the area-runtime label Apr 5, 2023
@mitchdenny
mitchdenny requested a review from JamesNK April 5, 2023 04:46
Comment thread src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterExtensions.cs Outdated
Comment thread src/Middleware/ConcurrencyLimiter/src/ConcurrencyLimiterMiddleware.cs Outdated
Comment thread src/Middleware/ConcurrencyLimiter/test/TestUtils.cs
@mitchdenny mitchdenny added the blog-candidate Consider mentioning this in the release blog post label Apr 5, 2023
@ghost

ghost commented Apr 5, 2023

Copy link
Copy Markdown

@mitchdenny, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

@mitchdenny

Copy link
Copy Markdown
Member Author

Adding blog candidate tag for this so we can let folks know that we are obsoleting this middleware.

Comment thread src/Middleware/ConcurrencyLimiter/sample/Startup.cs Outdated
@mgravell

mgravell commented Apr 5, 2023

Copy link
Copy Markdown
Contributor

see also src/Middleware/ConcurrencyLimiter/perf/Microbenchmarks/QueueEmptyOverhead.cs which is tripping CI failure

@Tratcher Tratcher added the breaking-change This issue / pr will introduce a breaking change, when resolved / merged. label Apr 5, 2023
@ghost ghost added the needs-breaking-change-announcement Indicates that breaking change announcement shuold be posted and linked to this PR label Apr 5, 2023
@ghost

ghost commented Apr 5, 2023

Copy link
Copy Markdown

Thanks for identifying a breaking change.

, after you commit this PR please take the following actions, as part of the breaking changes announcement process:
\n- [ ] Create an announcement issue by using the ASP.NET Core breaking change issue template.
\n- [ ] Link the breaking change announcement issue from this PR.
\n- [ ] Remove the needs-breaking-change-announcement label.

@Tratcher

Tratcher commented Apr 5, 2023

Copy link
Copy Markdown
Member

Please draft the announcement here in a PR comment for review.

We likely don't need to mention this in the blog, it's low usage.

@mitchdenny mitchdenny removed the blog-candidate Consider mentioning this in the release blog post label Apr 5, 2023

@JamesNK JamesNK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Remember to add a breaking change notification.

@mitchdenny

mitchdenny commented Apr 5, 2023

Copy link
Copy Markdown
Member Author

@Tratcher proposed wording for announcement:

In .NET 8.0 the ASP.NET Core team is deprecating the ConcurrencyLimiterMiddleware and associated methods and types (e.g. UseConcurrencyLimiter(...)). This package will be removed in .NET 9.0.

Developers requiring rate limiting capabilties should switch to the newer and more capable rate limiting middleware that was introduced in .NET 7.0 (e.g. UseRateLimiter(...)). The .NET 7.0 rate limiting API includes a concurrency limiter along with several other rate limiting algorithms that you can apply to your application.

For more information on rate limiting in ASP.NET Core see:
https://learn.microsoft.com/aspnet/core/performance/rate-limit

Should I just go ahead and create the breaking change issue?

@JamesNK

JamesNK commented Apr 5, 2023

Copy link
Copy Markdown
Member

That looks good for a summary. Go ahead. Here is an example of obsoleting an API announcement: aspnet/Announcements#458

btw, lock the issue on the announcement repo. I you don't, then random people comment on it and it sends out more notifications 😄

@mitchdenny
mitchdenny enabled auto-merge (squash) April 6, 2023 00:20
@mitchdenny
mitchdenny merged commit dce793f into dotnet:main Apr 6, 2023
@ghost ghost added this to the 8.0-preview4 milestone Apr 6, 2023
@gewarren

gewarren commented May 3, 2023

Copy link
Copy Markdown
Contributor

Is there a reason ASP.NET doesn't use custom obsoletion IDs like core .NET libraries? https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/obsoletions-overview

@ghost

ghost commented May 3, 2023

Copy link
Copy Markdown

Hi @gewarren. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@mitchdenny

Copy link
Copy Markdown
Member Author

/cc @davidfowl for his answers to this:

Is there a reason ASP.NET doesn't use custom obsoletion IDs like core .NET libraries? https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/obsoletions-overview

@ghost

ghost commented May 4, 2023

Copy link
Copy Markdown

Hi @mitchdenny. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 8, 2023
@Youssef1313 Youssef1313 removed the needs-breaking-change-announcement Indicates that breaking change announcement shuold be posted and linked to this PR label Aug 7, 2026
@Youssef1313

Copy link
Copy Markdown
Member

Removing needs-breaking-change-announcement.

This was announced in aspnet/Announcements#502

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares breaking-change This issue / pr will introduce a breaking change, when resolved / merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants