search

vutran1710 / PyrateLimiter

⚔️Python Rate-Limiter using Leaky-Bucket Algorithm Family
https://pyratelimiter.readthedocs.io
MIT License
338 stars 36 forks source link

Fix decorator/contextmanager usage with delay=True #29

Closed JWCook closed 3 years ago

JWCook commented 3 years ago

I found an issue with the decorator/contextmanager I previously added: after catching a BucketFullException and delaying, the delayed task will run without updating the bucket. This means the total delay time will be less than expected. For example, with 26 requests at 5 requests/second, the total delay time is ~4 seconds instead of 5. Same thing for non-concurrent async functions. For concurrent async functions, tasks won't get delayed correctly after the first delay.

The fix for this is to add a retry after the delay, until a task successfully acquires a bucket item. I updated the tests to check for this, and also added some more examples to the Readme.

vutran1710 commented 3 years ago

Can you bump the version to 2.3.1 as well?

JWCook commented 3 years ago

Done

vutran1710 commented 3 years ago

You forgot to change the version in the pyproject.toml file. Poetry will not be able to publish a new version without it.

JWCook commented 3 years ago

Okay, updated. Is there anywhere that the __version__ in pyrate_limiter/__init__.py is used, or can that be removed?

vutran1710 commented 3 years ago

I think it can be removed. but I never tried that lol. My guess is, it is the classic example of "if it doesn't break, don't fix it"

JWCook commented 3 years ago

Fyi, it looks like the conda-forge deployment worked as expected for 2.3.1: https://anaconda.org/conda-forge/pyrate-limiter 🎉

I also confirmed that the license file got included in the sdist on PyPI (pyrate-limiter-2.3.1.tar.gz).