tkem / cachetools

Extensible memoizing collections and decorators
MIT License
2.34k stars 163 forks source link

Extending `TTLCache`, `expire()` does not call `popitem()` #292

Closed mbrancato closed 2 months ago

mbrancato commented 11 months ago

Before reporting a bug, please make sure you have the latest cachetools version installed:

pip install --upgrade cachetools

Describe the bug For extending a class, the docs say all cache implementations call popitem() to evict items from the cache and indicate that overriding the popitem() can catch expiration from any class.

This does not work with TTLCache.

Expected result Overriding popitem() should be called when an item is expired.

Actual result The popitem() method is not called.

Reproduction steps

import time

from cachetools import TTLCache

class MyCache(TTLCache):
    def popitem(self):
        key, value = super().popitem()
        print('Key "%s" evicted with value "%s"' % (key, value))
        return key, value

c = MyCache(maxsize=20, ttl=2)
c["a"] = 1
time.sleep(1)
c["b"] = 2
time.sleep(4)
c["c"] = 3
GianlucaFicarelli commented 9 months ago

The report seems related to https://github.com/tkem/cachetools/issues/205 and https://github.com/tkem/cachetools/issues/103

If it's not possible or planned to add a notification callback when an item is removed because expired in TTLCache, it could be useful to make it more explicit in the docs.

tkem commented 9 months ago

Thanks @GianlucaFicarelli for pointing to the closed issues!

This use case seems to be more popular than I thought, so I might think about it again. Maybe expire() returning a list of evicted keys, so you can override it in a derived class and act on the result of the base class method. But don't expect anything too soon ;-)

But sure, the docs should definitely be improved in this respect.

Amedeo91 commented 4 months ago

Hello, I created this PR: https://github.com/tkem/cachetools/pull/320/files The goal of it is to perform some follow-up action when an element is going to expire, which is some how matching this use case. Let me know your thoughts