vutran1710 / PyrateLimiter

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

Sqlite Limitation #84

Closed probsJustin closed 1 year ago

probsJustin commented 2 years ago

Sqlite Limitation

probsJustin commented 2 years ago

I can also re-write this to not be dependent on np if it is needed. lmk if there is anything that should be changed.

JWCook commented 2 years ago

Thanks for looking into this!

The specific limitation here is the number of "host parameters" (? placeholders) that a SQLite query supports, which is 999 for SQLite < 3.32.0. It's much larger for the latest versions of SQLite, but I don't think any version of python ships with that yet. More details here: https://www.sqlite.org/limits.html

Numpy is a fairly heavyweight dependency, and this can be done without it easily enough. Here's an example (used for the same purpose, splitting up a large SQLite statement): https://github.com/requests-cache/requests-cache/blob/b9dbc008be7274b6cc0d45078b3334249df1a530/requests_cache/_utils.py#L10-L14

You can copy that, and split statements into chunks of 999 keys.

probsJustin commented 2 years ago

I went through and made the change, let me know if it looks good or if there is anything else that needs to be changed - I reran it on my end for the use case and it seems to be working

probsJustin commented 2 years ago

Sorry, it took a minute for me to hit this error again, I don't think this fixes it the same way that np did - I will take a look at it and see what I did wrong.

JWCook commented 2 years ago

Can you post the full traceback you get?

Also, committing the transaction after every 50 keys is going to be slower than necessary. We'll want to fit as many keys as possible per statement, which should be 999, and then call .commit() once, after all the statements are made.

probsJustin commented 2 years ago

I am not sure that I will hit this limit again in the project I am working on, I will let you know but it will take about a week till I can get to that point again

JWCook commented 1 year ago

I think the best way to handle that would be to add a unit test to tests/test_sqlite_bucket.py that adds a few thousand items and then removes them all with one call to SQLiteBucket.get().

Let me know if you would like any help with that.

vutran1710 commented 1 year ago

@probsJustin @JWCook so whats the status of this PR?

JWCook commented 1 year ago

I can take it from here and make the changes I mentioned above.

JWCook commented 1 year ago

@vutran1710 You can close this PR; finished changes are in #87