feat(quotas): Add ability to use namespace in non-global quotas. #3090
Conversation
|
|
||
| let rate_limiter = build_rate_limiter(); | ||
|
|
||
| for i in 0..10 { |
There was a problem hiding this comment.
You're effectively having two loops in one, and splitting it increases readability:
for i in 0..5 {
assert_eq!(
rate_limits[0].reason_code,
Some(ReasonCode::new("ns: None"))
);
}
for i in 0..5 {
assert_eq!(rate_limits, vec![]);
}(Same with the loop below)
There was a problem hiding this comment.
hmm i get your point but it also makes it a bit less clear the separation of the loop with and without the namespace, and we'd also have to duplicate the let ratelimits = ... block more.
I agree it's not as clear as can be though, I uploaded some changes.
- added comments
- the 'quota_limit' variable should be more clear
- flipped the if/else statements
There was a problem hiding this comment.
This is probably my personal opinion, and not going to block the PR for that, but this is one case in which I'm ok with some code duplication given I find less cognitive load and thus more readable.
|
|
||
| let rate_limiter = build_rate_limiter(); | ||
|
|
||
| for i in 0..10 { |
There was a problem hiding this comment.
This is probably my personal opinion, and not going to block the PR for that, but this is one case in which I'm ok with some code duplication given I find less cognitive load and thus more readable.
#2716
With this PR, we can also use namespace in non-global quotas.
The reason for this PR was the requirement to be able to ratelimit based on namespace per org, but in reality it's pretty flexible so we can do namespace per project as well.