Closed ruslandoga closed 10 months ago
Hi! Thanks for opening this PR, and sorry we haven't seen it until now. I'll try to look at this soon, probably over the winter break.
cc @epinault
Wow yes sorry I ll take a look at it this week but same I never got a notification on this either . I ll have to check my settings cause I usually get notified
@egeersoz @epinault
Thank you for the comments! I'm going to split this PR into multiple non-breaking ones, some to update CI, some to update ETS backend without the breaking changes, and then a few with the breaking changes.
It seems like no matter where I start: removing application.ex and global supervisor, removing global config and app env lookups, removing poolboy, -- it all results in breaking changes every time :)
And every time I end up with something similar to this:
# instead of config, each backend is defined as a module
defmodule MyApp.RateLimit do
use Hammer, backend: :ets, table: __MODULE__ # that would be the default table
end
defmodule MyApp.AnotherRateLimit do
use Hammer, backend: Hammer.Backend.Redis, pool: __MODULE__ # that would be the default pool
end
# each backend can be started separately and be configured at start-up
MyApp.RateLimit.start_link(cleanup_period: :timer.seconds(10))
MyApp.AnotherRateLimit.start_link(url: "...", ...)
# module calls remove the need for runtime backend lookups
{:allow, _} = MyApp.RateLimit.hit("some-key", _scale = :timer.seconds(60), _limit = 3)
1 = MyApp.RateLimit.get("some-key", _scale = :timer.seconds(60), _limit = 3)
MyApp.RateLimit.set("some-key", _scale = :timer.seconds(60), 3)
MyApp.RateLimit.wait("some-key", _scale = :timer.seconds(60), _limit = 3) # = Process.sleep(next_bucket())
MyApp.RateLimit.reset("some-key", _scale = :timer.seconds(60)) # = set(...,...,0)
# etc.
So I'm thinking, would it be OK to just resume this PR and implement this version I always end up with?
sure!
fixes #71
I'm going to leave some comments inline explaining the changes.