uber-go / ratelimit

A Go blocking leaky-bucket rate limit implementation

MIT License

4.32k stars 300 forks source link

Use actively maintained clock library #89

Closed storozhukBM closed 2 years ago

storozhukBM commented 2 years ago

It looks like our clock mocking library is supper old and archived by author. It can be a cause of our test instabilities. This PR replaces it with drop-in replacement that is actively maintained.

codecov[bot] commented 2 years ago

Codecov Report

Merging #89 (af3a0fd) into main (0fa90d2) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #89   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           70        70           
=========================================
  Hits            70        70

Impacted Files	Coverage Δ
ratelimit.go	`100.00% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0fa90d2...af3a0fd. Read the comment docs.

rabbbit commented 2 years ago

There's a bit of a larger history here.

We used to have an internal fork of the andres-erbsen clock to work around a bug there. However, our own fork ended up having two bugs (not sure if they were present in anders-erbsen). See #39.

At the time I looked at history between anders-erbsen and the clock you're proposing, both had commits that were not being present on the other one, so I didn't move back to the other fork. And the bobjohnson one had a gap in pushes before the 2021 push by djmitche.

That being said, at this point, you're right that it probably makes sense to just use the "more actively" and hope we don't hit more bugs :]