alisaifee / limits

Rate limiting using various strategies and storage backends such as redis & memcached
https://limits.readthedocs.org
MIT License
423 stars 58 forks source link

mypy reports incompatible type in rate limiters #88

Closed russellcardullo closed 2 years ago

russellcardullo commented 2 years ago

Hello! I've recently updated one of my projects from limits 2.0.3 to 2.1.1 and ran into an issue where mypy reports incompatible types for MovingWindowRateLimiter and FixedWindowRateLimiter. A minimal reproducer:

import limits

storage = limits.storage.storage_from_string("memory://")
strategy = limits.strategies.MovingWindowRateLimiter(storage)

With this file and running mypy 0.930 I get:

error: Argument 1 to "MovingWindowRateLimiter" has incompatible type "Union[limits.storage.base.Storage, limits.aio.storage.base.Storage]"; expected "limits.storage.base.Storage"

I could also reproduce that by running mypy on the tests in this repo: python3 -m mypy tests/test_strategy.py.

Any ideas what I could look at next?

Thanks!

alisaifee commented 2 years ago

Looks like this is just a bug introduced by storage_from_string returning either the aio or sync version of the storage.

alisaifee commented 2 years ago

Could you try this commit to see if it resolves the issue? https://github.com/alisaifee/limits/commit/4265c719de70172fabeadc966e0b5eac0d1345dc

alisaifee commented 2 years ago

Should be resolved in 2.2.0

russellcardullo commented 2 years ago

Thanks! Just confirmed 2.2.0 fixes it.

russellcardullo commented 2 years ago

Hi! I've started seeing this again starting in version 2.5.2 and all newer versions. I used the same reproducer in the description and got:

error: Argument 1 to "MovingWindowRateLimiter" has incompatible type "Union[limits.storage.base.Storage, limits.aio.storage.base.Storage]"; expected "limits.storage.base.Storage"

Tried 2.5.1 and the problem went away.

alisaifee commented 2 years ago

Ouch, sorry about that. 2.5.2 was actually supposed to improve type safety 😿

alisaifee commented 2 years ago

Fixed in 2.5.4 and 2.6.1. Sorry about the regression.