Skip to content
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

[RateLimiter] Fix minor issue when sharing windows between Limiters #38566

Closed
wants to merge 0 commits into from

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Oct 14, 2020

Q A
Branch? 5.x
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

If I start using my custom Limiter, then change back to FixedWindowLimiter, then my cache might contain a value that FixedWindowLimiter does not support.

This PR makes sure that we handle such switch.

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Oh, very nice catch!

Can you do the same change in TokenBucketLimiter?

@Nyholm
Copy link
Member Author

Nyholm commented Oct 14, 2020

Thank you.

I've updated the PR

@nicolas-grekas nicolas-grekas added this to the 5.2 milestone Oct 14, 2020
@nicolas-grekas nicolas-grekas changed the title Fix minor issue when sharing windows between Limiters [RareLimiter] Fix minor issue when sharing windows between Limiters Oct 14, 2020
@nicolas-grekas nicolas-grekas changed the title [RareLimiter] Fix minor issue when sharing windows between Limiters [RateLimiter] Fix minor issue when sharing windows between Limiters Oct 14, 2020
@nicolas-grekas
Copy link
Member

Thank you @Nyholm.

nicolas-grekas added a commit that referenced this pull request Oct 14, 2020
…olm)

This PR was squashed before being merged into the 5.x branch.

Discussion
----------

Fix minor issue when sharing windows between Limiters

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

If I start using my custom Limiter, then change back to `FixedWindowLimiter`, then my cache might contain a value that `FixedWindowLimiter` does not support.

This PR makes sure that we handle such switch.

Commits
-------

e9ac971 Fix minor issue when sharing windows between Limiters
@fabpot fabpot mentioned this pull request Oct 14, 2020
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.

None yet

4 participants