Block writing cache in cached decorator if custom condition is not satisfied

[VitalyPetrov]

I m using the  aiocache.cached decorator with wraps the certain processing function. The setup is as following:

import aiocache


aiocache.caches.add(
    "cache-alias",
    {
        "cache": "aiocache.RedisCache",
        {
            <cache-config-data>
        }
    },
)


@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    ttl=ttl,
)
async def processing(...) -> int:
    ...

The problem is -- can i pass any argument to cached decorator in order to avoid cache writing for processing(...) return?
It seems like the only way to perform it is to throw exception inside the processing(...) function if the result is not appropriate one. I m looking for more generic way to do the same stuff.

For example, pass some Callable object to cached.__call__() returning the bool var meaning caching the results. Something similar to:

def checker(value: Any) -> bool:
    ...

@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    value_checker=checker,
    ttl=ttl,
)
async def processing(...) -> int:
    ...

[Dreamsorcerer]

Sounds plausible. Can you give an example use case of what you'd use this for?

[VitalyPetrov]

The real setup is quite more complex so I d better describe the more simple one.

For instance you want to request some external API which perform computational-intensive calculation inside and then return the result in JSON. Sure you want to call that external API as rare as possible so you use caching:

@aiocache.cached(
    alias="local-cache",
    key_builder=some_key_builder,
    ttl=ttl,
)
async def ask_ranking_worker(*args, **kwargs) -> tuple[int | None, Any]:
     # <request-external-api-and-get-the-responce>
    return err, result
    
    
async def main() -> None:
     ...
     err, result = await ask_ranking_worker(*args, **kwargs)
     ...
     return

In some cases ask_ranking_worker() can't work properly so the err is not None. But the result of that calculation is cached and you can't make a retry before the key is expired or you manually delete it. The only way to perform it is to rise the exception / error inside the ask_ranking_worker function:

@aiocache.cached(
    alias="local-cache",
    key_builder=some_key_builder,
    ttl=ttl,
)
async def ask_ranking_worker(*args, **kwargs) -> tuple[int | None, Any]:
     # <request-external-api-and-get-the-responce>
     
     if err is not None:
         raise RuntimeError(f'Worker failed with an error: {err}')
     
    return err, result

That workaround prevents the result to be cached and makes retries possible

[VitalyPetrov]

Sure you can do it with direct cache.set() and cache.get() but still...

[Dreamsorcerer]

Hmm, it looks a bit unpythonic. Wouldn't it make more sense to raise err or a new exception containing whatever err is?

Generally, Python code does not follow the design of returning an error state (that's something more common in C or Go, for example). So, it feels like your second example would be better, but without returning err at all.

[VitalyPetrov]

Well, yeah, this code is not pythonic at all, I just tried to demonstrate the issue as simple as possible 😄

I can rewrite it without error status:

@aiocache.cached(
    alias="local-cache",
    key_builder=some_key_builder,
    ttl=ttl,
)
async def ask_ranking_worker(*args, **kwargs) -> Any:
     # <request-external-api-and-get-the-responce>
     
     if not result:
         raise RuntimeError(f'Worker failed, no result get')
     
    return result
    
    
async def main() -> None:
     ...
     result = await ask_ranking_worker(*args, **kwargs)
     ...
     return

The problem remains the same -- empty result wouldn't be cached only using that workaround with error rising.

[Dreamsorcerer]

Right, I'm just trying to understand a real world use case, to see what the best solution would be. I can think of a trivial example, but I'm not sure what the real use case is currently.

In that example, it might work as you'd want, because I think the cache is not saved if the result is None (I think there's another issue open discussing whether that's desired or not).

[Dreamsorcerer]

I guess I'm wondering if there's a reason you'd not want to cache a result which is not None (or atleast falsy), and whether you'd know which kind of result shouldn't be cached before calling the function..

[VitalyPetrov]

In some cases the empty result (or more generally value_checker(result) == False) should be saved while in another ones it shouldn't. The feature request is to add that value_checker callable to cached.__call__() in order to control that behaviour in a more straightforward way. However I'm not insist and if you think that throwing the exception is more generic -- ok, that fine 😄

[Dreamsorcerer]

Well, if it's only a question of whether it is falsy or not, we could just have a simple cache_falsy=False option (which would also resolve that other issue). I'm just wondering if there's any real case for having a callable; it's more flexible, but would it actually get used for anything other than testing if something is falsy?

[VitalyPetrov]

Well it's an interesting question )

I ve start thinking about that feature (cache_falsy) when I realised I have to handle multiple different exceptions which makes my codes far less readable. Then I rewrite it in a go-style code (means the function to be cached returns (err, res) tuple). It made the code prettier but allowed results with err is not None to be cached.

On my opinion cache_falsy=False is a bit limited as it requires for your result to have __bool__() method which is quite unobvious. From that point of view the value_checker(result: Any) -> bool callable is more flexible as it makes your code more explicit.

[Krukov]

I can give a few examples where callable takes place:
We have a function that return a list of objects, for example user's posts

1. we don't want to cache if it gets too match posts

def checker(value: list[Post]) -> bool:
    return len(value) < 100

@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    value_checker=checker,
    ttl=ttl,
)
async def get_posts(user: User) -> list[Post]:
    ...

2. we don't want to cache if at least one post have a mark:

def checker(value: list[Post]) -> bool:
    return not any(post.skip_cache for post in posts)

@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    value_checker=checker,
    ttl=ttl,
)
async def get_posts(user: User) -> list[Post]:
    ...

3. Another option to have a callable checker is to pass calling args, kwargs

USERS_WITHOUT_CACHE = [1, 2,  3]
def checker(result: Any, args, kwargs) -> bool:
    return kwargs["user"].id not in USERS_WITHOUT_CACHE

@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    value_checker=checker,
    ttl=ttl,
)
async def get_posts(*, user: User) -> list[Post]:
    ...

The last example can be replaced with:

USERS_WITHOUT_CACHE = [1, 2,  3]

@aiocache.cached(
    alias="cache-alias",
    key_builder=some_key_builder,
    value_checker=checker,
    ttl=ttl,
)
async def _get_posts_cache(user: User) -> list[Post]:
    return await _get_posts(user)

async def _get_posts(user: User) -> list[Post]:
    ...
   
async def get_posts(user: User) -> list[Post]:
    if user.if in USERS_WITHOUT_CACHE:
        return await _get_posts(user)
    return await _get_posts_cache(user)

Much uglier than if aiocache provided the ability to control caching condition.

[Dreamsorcerer]

Hmm, I'd probably expect 1 to use paging rather than return such large results (though maybe if you're using a memory cache you'd want to cache small responses, so maybe it does work..), and 3 could easily be done with an explicit cache bypass (e.g. get_posts.bypass_cache(user) which I've suggested in #625).

2 seems more reasonable, though it's not clear exactly why you'd skip the cache, but I can perhaps imagine something to avoid sensitive data being stored in the cache or specific privileged accounts that need to always have up-to-date results maybe... though I'm still struggling to think of an example where I wouldn't know this information before making the call.

So, I think the callable solution is probably fine then. Anybody want to work on making a PR to add it?

[VitalyPetrov]

I'd like to try. Could you please provide me with your git flow?

[Dreamsorcerer]

Just generally open a PR to master is fine. Once a PR is open, it's best to avoid rebases (as it makes it harder to review), the PR will be squash committed anyway.

[Krukov]

@VitalyPetrov if you are looking for something that already support that kind of functionality in advanced way, check here. Sorry for advertisement, by I don't get why aiocache can't be as much flexible as it possible.