Uh oh!
There was an error while loading. Please reload this page.
[release/7.0] Rename request count to permit count on System.Threading.RateLimiting - #76623
Conversation
ghost
commented
Oct 4, 2022
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
ghost
commented
Oct 4, 2022
Tagging subscribers to this area: @mangod9 Issue DetailsBackport of #76558 to release/7.0 Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
carlossanlop
commented
Oct 4, 2022
@BrennanConroy can you please provide a code review sign-off? |
BrennanConroy
commented
Oct 5, 2022
@carlossanlop good to merge? |
carlossanlop
commented
Oct 5, 2022
CI failures:
Signed off, approved by Tactics. Ready to merge. |
Backport of #76558 to release/7.0
/cc @BrennanConroy@AliKhalili
Customer Impact
API is inconsistent with other rate limiters (ConcurrencyLimiter) and other APIs (the options refer to permits not requests), and not what was approved during API review. This can cause confusion. And we can't change this later as it is a source breaking change.
runtime/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/SlidingWindowRateLimiterOptions.cs
Line 36 in dd6696a
Testing
N/A
Risk
Code using System.Threading.RateLimiting in previews, RC1, or RC2 may fail to compile after this change if using parameter names. e.g.
limiter.AttemptAcquireCore(requestCount: 3);Fortunately, it'll be very easy to fix broken code, change
requestCounttopermitCount.