Make int64 based atomic ratelimiter default #101
Conversation
storozhukBM
changed the title
Codecov Report
@@ Coverage Diff @@
## main #101 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 95 98 +3
=========================================
+ Hits 95 98 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| sleepDuration := time.Duration(newTimeOfNextPermissionIssue - now) | ||
| if sleepDuration > 0 { | ||
| t.clock.Sleep(sleepDuration) | ||
| return time.Unix(0, newTimeOfNextPermissionIssue) | ||
| } |
There was a problem hiding this comment.
Okay, now I'm confused again :)
- you removed very similar lines in Fix no slack option for int64 based option #95 (https://github.com/uber-go/ratelimit/pull/95/files#diff-30af80b2f0055e05bdc7d6ecea1b49d55f1ef946c381a1604f8e03d58789cd3fL85)
- Fix test approach for detecting issues #93 which "fixes the time" - which I assumed is the goal-state - doesn't have this change.
Why re-introduce this now?
There was a problem hiding this comment.
4 month ago, you added a new test Example_default that checks the slack behaviour of the default ratelimiter.
It turns out, atomicLimiter returns now if it doesn't sleep, so I decided to sync this behaviour in atomicInt64Limiter.
you removed very similar lines in #95
Yes, these lines are similar, but slightly different in the current PR we return now if we don't sleep, and this is the only behaviour change, everything else works as it worked before.
#93 which "fixes the time" - which I assumed is the goal-state - doesn't have this change.
In #93 atomicLimiter is still a default implementation, so I didn't know about the difference between behaviours so far.
No description provided.