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

Replicate counter identity rules #13

Closed
4 of 7 tasks
neongreen opened this issue Sep 17, 2019 · 0 comments · Fixed by #51
Closed
4 of 7 tasks

Replicate counter identity rules #13

neongreen opened this issue Sep 17, 2019 · 0 comments · Fixed by #51
Assignees

Comments

@neongreen
Copy link
Contributor

neongreen commented Sep 17, 2019

Looks like we do not need to remove counters when rules are reloaded. The behavior of lyft/ratelimit is as follows:

  • When rules are reloaded, existing counters are not touched.
  • When the reloaded rules are the same but one limit has changed, the counter is not reset, and the new limit is taken into account.
  • When the reloaded rules are the same but one unit has changed, a new counter is used, and the old counter is not removed.

So we need to:

  • add ratelimit unit to the CounterKey
  • document in code that counters don't need to be removed, and explain why
  • document the behavior in README/elsewhere
  • write tests
@neongreen neongreen changed the title Remove counters when rules are reloaded, or explain why we don't need to do anything Removing counters when rules are reloaded Sep 27, 2019
@neongreen neongreen changed the title Removing counters when rules are reloaded Replicate counter identity logic Sep 27, 2019
@neongreen neongreen changed the title Replicate counter identity logic Replicate counter identity rules Sep 27, 2019
@mdimjasevic mdimjasevic self-assigned this Oct 7, 2019
mdimjasevic added a commit that referenced this issue Oct 15, 2019
* Closes #13: replicates the rule counter logic from lyft/ratelimit with regard to rule reloading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants