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

Caching exceptions. #138

Open
WloHu opened this issue Aug 19, 2019 · 10 comments
Open

Caching exceptions. #138

WloHu opened this issue Aug 19, 2019 · 10 comments

Comments

@WloHu
Copy link

WloHu commented Aug 19, 2019

I have a usecase where decorated function will raise exceptions for some values and I would like these exceptions to be cached and raised again.

I opened this issue to know if something like this is already implemented or was thought through.

So far what I've though about this is:

  • the best approach would be to decorate cache decorators, instead adding something like cache_exceptions=True flag to all of them;
  • caching traceback info is a huge NO, so the only way would be to store exception types, w/ or w/o arguments, and create and raise new exception instance, the con is that traceback info will be lost on consecutive calls.

Your thoughts?

@tkem
Copy link
Owner

tkem commented Aug 19, 2019

Kind of thought about that a long while ago, but didn't know how to deal with transient (e.g. network) related exceptions vs. persistent ones (e.g. this URL does not exist). To "decorate cache decorators" as you put it, instead of adding yet another flag, would clearly be a good starting point. Regarding tracebacks... I'm interested, what makes you think these are a "huge NO"?

@tkem
Copy link
Owner

tkem commented Aug 19, 2019

As for decorating decorators, the only thing I'd come up would probably be to cumbersome to use:

@raise_cached_exceptions(...)
@cached(...)
@cache_exceptions(...)
def foo(...)

where @cache_exceptions would catch exceptions and convert them to some special return value/type, and @raise_cached_exceptions would look for that value/type and raise it. Argh...
I think it would also be nice to be able/forced to configure exception base types to be handled that way, e.g. catch_exceptions=(TypeError, OSError) or something like that, instead of having a "catch-all" clause.

@WloHu
Copy link
Author

WloHu commented Aug 19, 2019

Good point with "catch-all".

In regards to looks I though about:

@cached(cache=IncludeExceptions(LRUCache, reraise=True, maxsize=32))
def foo(...):

Where 1st argument is cache type and kwargs like reraise are passed to IncludeExceptions and unmatched ones are used to initialize cache type. Underneath it could dynamicly create subclass of passed cache type and initialize itself.

Do you think exceptions should be kept in separate "container" than the rest of cached values? My first idea was that all cached values and exceptions sholud share the same container.

The traceback object issue I tought of is related to circular dependencies caused by it (https://stackoverflow.com/a/1658335). Keeping them in cache may cause excessive memory usage and make it harder for garbage collector to find cycles.

@tkem
Copy link
Owner

tkem commented Aug 19, 2019

Good points! Have to think about it. Don't expect any comments before next weekend (work, life, etc.) ;-)

@tkem
Copy link
Owner

tkem commented Aug 19, 2019

Anyway, having a @cached(..., exceptions=(...)) doesn't seem to be that far-fetched any more to me, though ;-)

@WloHu
Copy link
Author

WloHu commented Aug 19, 2019

Indeed, this looks better.

@bra-fsn
Copy link

bra-fsn commented Oct 2, 2019

Having the same issue I would say 👍 for the selective approach. This way the user can choose between temporary/permanent exception and list them as appropriate.

@bra-fsn
Copy link

bra-fsn commented Oct 2, 2019

BTW, I would be happy to get the same exception reraised as the original. So this should be definitely configurable.

@tkem tkem added this to the v4.0.0 milestone Nov 25, 2019
@tkem tkem removed this from the v4.0.0 milestone Dec 12, 2019
@hellocoldworld
Copy link

Hello! @tkem are you currently working on this? Would you be happy to recieve a PR regarding this?

@tkem
Copy link
Owner

tkem commented Jul 30, 2022

It's quite embarrassing for me to answer this after more than half a year of inactivity, but somehow this wasn't on my radar...
Currently, I have no plans including this in cachetools, due to limited resources and other items in the backlog, sorry!
But I'd like to keep this open for future reference.

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

No branches or pull requests

4 participants