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

Add support for datetime.timedelta for TTLCache TTL #216

Closed
john-bodley opened this issue Aug 2, 2021 · 3 comments
Closed

Add support for datetime.timedelta for TTLCache TTL #216

john-bodley opened this issue Aug 2, 2021 · 3 comments
Milestone

Comments

@john-bodley
Copy link

john-bodley commented Aug 2, 2021

I realized that cachetools is not typed, but I was wondering whether the ttl argument for the TTLCache class should also support a datetime.timedelta object for specifying (and preferably storing) the TTL, i.e.,

from datetime import timedelta


class TTLCache(Cache):
    """LRU Cache implementation with per-item time-to-live (TTL) value."""

    def __init__(self, maxsize, ttl: Union[int, timedelta], timer=time.monotonic, getsizeof=None):
       self.__ttl = timedelta(seconds=ttl) if isinstance(ttl, int) else ttl
       ...

The reason I ask is using an int for encoding the TTL is somewhat difficult to grok as i) it is not apparent what the units are, and ii) larger TTL become hard to comprehend resulting in people including a comment for readability like,

@cached(cache=TTLCache(maxsize=1024, ttl=86400)) # 24 hours

or

@cached(cache=TTLCache(maxsize=1024, ttl=60 * 60 * 24)) # 24 hours

rather than simply writing

@cached(cache=TTLCache(maxsize=1024, ttl=timedelta(hours=24)))
@tkem
Copy link
Owner

tkem commented Aug 3, 2021

The ttl argument has to be compatible with the result of the timer function, at least so that it can be added to and the result compared with timer(), so you could try for example (not tested)

import cachetools
import datetime

c = cachetools.TTLCache(maxsize=10, ttl=datetime.timedelta(seconds=10), timer=datetime.datetime.now)

Typings support for cachetools is provided by typeshed, but expressing these kinds of dependencies isn't so easy, apparently, so ttl is just float there. Maybe you can contribute to improve the situation over there?

@tkem
Copy link
Owner

tkem commented Aug 9, 2021

However, this would probably be a nice addition to the TTLCache docs, to show proper use of the timer argument.

@tkem
Copy link
Owner

tkem commented Sep 28, 2021

Unit tests should also be added for this.

@tkem tkem closed this as completed in c0162c5 Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants