Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting maxsize=None on TTLCache still checks the size in __setitem__ #328

Open
sedrik opened this issue Jan 24, 2025 · 5 comments
Open
Assignees
Labels

Comments

@sedrik
Copy link

sedrik commented Jan 24, 2025

Describe the bug

>>> from cachetools import TTLCache
>>> c = TTLCache(maxsize=None, ttl=5)
>>> c["test"] = 123
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sedrik/.cache/pypoetry/virtualenvs/pymarly-P2K_hR8A-py3.10/lib/python3.10/site-packages/cachetools/__init__.py", line 425, in __setitem__
    cache_setitem(self, key, value)
  File "/home/sedrik/.cache/pypoetry/virtualenvs/pymarly-P2K_hR8A-py3.10/lib/python3.10/site-packages/cachetools/__init__.py", line 74, in __setitem__
    if size > maxsize:
TypeError: '>' not supported between instances of 'int' and 'NoneType'

Expected result
I would expect the call c["test"] = 123 to succeed

Actual result
TypeError: '>' not supported between instances of 'int' and 'NoneType'

Reproduction steps
See bug description

@sedrik sedrik added the bug label Jan 24, 2025
@tkem
Copy link
Owner

tkem commented Jan 26, 2025

None is not valid as maxsize in any of the Cache classes, please point me to the docs if they say differently.
The only place None is allowed is in the cachetools.func decorators, for compatibility with lrucache.
To get the behavior you probably want, please use maxsize=math.inf or something similar.

@tkem tkem closed this as completed Jan 26, 2025
@sedrik
Copy link
Author

sedrik commented Jan 26, 2025

Hi yes you are right, it is not documented. I read the code for the decorator and just assumed it would work the same for the regular class. Will use maxsize=math.inf 👍

@tkem
Copy link
Owner

tkem commented Jan 26, 2025

Maybe I also should state explicitly that a number is required, or in the func decorators, add a comment that this is kind of special for compatibility only (you may note that it mostly uses plain old dict if None is passed).

At least with the typeshed typings it says maxsize: float...

@tkem tkem added docs and removed bug labels Jan 26, 2025
@tkem tkem reopened this Jan 26, 2025
@sedrik
Copy link
Author

sedrik commented Jan 26, 2025

Both documentation changes are probably good improvements. In my case I have a single endpoint I want to limit to roughly 1 RPS so I will never hit the max size so that is why I did not want to set one (or set it infinitely high).

Sorry for the noise and thank you for your hard work on this project :)

@tkem
Copy link
Owner

tkem commented Jan 26, 2025

@sedrik: Thanks for reporting (properly, using the template and reproducible code and all), even if it's "just" something that might be improved in the docs.

These are the things really important to users, not some supa-dupa new high-speed implementation in Rust or whatever, so I don't consider this "noise" at all ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants