Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[RateLimiter] Fix results on last token consume #53064

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

Open
wants to merge 1 commit into
base: 5.4
Choose a base branch
Loading
from

Conversation

ERuban
Copy link
Contributor

@ERuban ERuban commented Dec 13, 2023

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
License MIT

This PR is a result of the discussion in PR #52835 (@wouterj)

It fixes wrong retryAfter values for fixed_window and token_bucket policies and some related things.

PS: Also seems there is one more bug with the token_bucket - I'm sure that the TokenBucket::$timer should be used somehow in the Rate::calculateTimeForTokens() - it calculates time from now at this moment.

@ERuban
Copy link
Contributor Author

ERuban commented Dec 13, 2023

CI is red, but I'm not sure that it related to my changes.

@ERuban ERuban force-pushed the fix_last_token_usage branch from cab4fab to ac6348a Compare December 17, 2023 13:20
@VoodooPrograms
Copy link

What is the status of this PR? Since half of the year nothing push it closer to live.

@fabpot fabpot modified the milestones: 5.4, 6.4 Dec 14, 2024
@fabpot
Copy link
Member

fabpot commented Mar 29, 2025

@wouterj Can you have a look at this one?

@OskarStark OskarStark changed the title [RateLimiter] Fix results on last token consume. [RateLimiter] Fix results on last token consume Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.