abersheeran / asgi-ratelimit

A ASGI Middleware to rate limit
Apache License 2.0
306 stars 13 forks source link

Another redis implementation #10

Closed euri10 closed 3 years ago

euri10 commented 3 years ago

fixes https://github.com/abersheeran/asgi-ratelimit/issues/8

ok this is a WIP but I think it is a nice implementation, there is still some work to polish it (most importantly is the fact that the block feature happens to works by accident I think, but I have the feeling it can get added to the script easily), remove debug stuff etc but I wanted to show it to maybe talk with concrete examples about it.

the key concept here is that there is just one script, that avoids calling redis twice per round-trip (one for increase limits, one for decrease) so it should be faster than the current implementation.

it implements a sliding window with sorted sets : I've been inspired by https://levelup.gitconnected.com/implementing-a-sliding-log-rate-limiter-with-redis-and-golang-79db8a297b9e that does the same but only for a single time window, this one is adapted to make it possible to run multiple independent sliding windows

let me know if that's something worth continuing. note also the failing tests are on the current redis implementation because of the /multiple tests added, it passes fine on the new implementation

abersheeran commented 3 years ago

Looks like a good change!

euri10 commented 3 years ago

I implemented requested changes, added test for multiple rules only for the SlidingRedisBackend so that it does not fail on the RedisBackend and commented out the logging, just need to know if you want to keep it commented in the code or pasted here for future reference, it helped me a lot debugging the lua script so it might prove useful to someone elese in the future.

euri10 commented 3 years ago

I had to poetry update to please the CI that failed in https://github.com/abersheeran/asgi-ratelimit/pull/10/commits/80157a0ebbc631d3d065b43a0ab39ee35f648902

this explains some changes in poetry.lock and black formatting since it upped the version

abersheeran commented 3 years ago

I had to poetry update to please the CI that failed in 80157a0

this explains some changes in poetry.lock and black formatting since it upped the version

😀It doesn't matter, this is an occasional issue on GitHub. In fact, re-running CI should bring it back to normal.

abersheeran commented 3 years ago

Do you still want to modify anything? If not, I will merge this PR.

euri10 commented 3 years ago

Do you still want to modify anything? If not, I will merge this PR.

let's roll !