1. Michael Bayer
  2. dogpile.cache
  3. Issues
Issue #24 new

Cache invalidation for class or instance methods

David Beitey
created an issue

When attempting to invalidate or set a value for a class or instance method that's being cached via CacheRegion.cache_on_arguments, the first argument passed to the decorated function's invalidate or set methods is ignored. For example, if I have

class Foo(object):

    def do_something(self, text):
        return text + 'dummy'

then if one wants to clear or set the cache for this method, they need to do this:

foo = Foo()
cached = foo.do_something('asdf')
foo.do_something.invalidate(anything_goes, 'asdf')
foo.do_something.set('value', anything_goes, 'asdf')

as the first argument passed into either function is ignored as part of the cache key. This is probably expected behaviour for now, but I think the situation should be documented either way.

Since the first argument going into those invalidate/set is ignored regardless by the key generator, I think it would be easier to have the key generator use all arguments if the decorated method belongs to a class or instance. This would be far less prone to error as I've found when trying to clear such a cached method & forgetting the first argument.

Comments (9)

  1. Michael Bayer repo owner

    Since the first argument going into those invalidate/set is ignored regardless by the key generator, I think it would be easier to have the key generator use all arguments if the decorated method belongs to a class or instance.

    OK I'm having trouble determining what you mean here, but I think you mean, have the key generator accept all the args, and move the "self/cls" logic up to the CacheRegion. If we just remove the "self/cls" logic entirely then it still fails to be consistent between the two calls - the issue remains, and the behavior of the default key function changes as well which would dramatically alter the behavior of caching (though we are pretty much stuck with a backwards incompatible change here in any case).

    But moving the self/cls logic means, now the function_key_generator's function never gets to see "cls" or "self", which means someone using a custom function_key_generator doesn't get to see what's going on either.

    What would be best would be if we could get the ".invalidate" and ".set" functions we tack on there to receive cls/self in the same way as the parent method does. then the default function_key_generator works and a user who specifies a custom one gets to see the cls/self coming in as an argument too. I'm not sure at the moment how to achieve that though, as we would need to get at the clsmethod/instancemethod wrapper applied to the function after the fact.

  2. David Beitey reporter

    Yep, that's the gist of it. My main confusion was that calling either .invalidate or .set on a cached method needed to receive all of the same arguments that the original method did -- including something fulfilling the role of cls/self as the first argument. I didn't immediately realise this, however.

    My original thinking was for the ".invalidate" and ".set" functions to just determine if they were attached to a class/instance method and then generate the key using all arguments passed if so (rather than ignoring the first argument), or using standard logic otherwise. However, you're right in your suggestion in the last paragraph - that sounds flexible (especially if someone is writing a custom key generator).

    Otherwise, if nothing else in the meantime, just a line or two of documentation about including that first argument would help others avoid my mistake.

  3. Jabberwok

    Yes, this is confusing. I am not sure that 'cls' should be considered for has_self, because it will never change from call to call, so it is not a variable parameter actually and doesn't have anything to do with caching by values.

  4. Morgan Fainberg

    I'm wondering if we could do some metaprogramming and solve the issue with a configurable (e.g. make it so you automatically get the self/cls from the decorated method to the invalidate and set). I think I can see a mechanism to allow this if a "dogpile.CachingMeta" class was added to the object that has decorated methods. Similar to how ABCMeta works. I'm not sure this thought-train is fully baked though.

  5. Michael Bayer repo owner

    it's a thorny problem, and in general my small amount of actual use with the cache decorator has already left me unsatisfied re: key generation in any case, but it's not the highest priority for me right now. as always id like to make the system more open ended to support end-user recipes at least.

  6. Melvii Ts

    How about to solve the problem like this.

    From 212e46bba51251ac02c76e8822fdd88fe7c3cba2 Mon Sep 17 00:00:00 2001
    From: layz <layzerar@gmail.com>
    Date: Thu, 19 Dec 2013 10:21:03 +0800
    Subject: [PATCH] Fix issue #24
     dogpile/cache/region.py | 13 +++++++------
     dogpile/cache/util.py   |  9 ++++++++-
     2 files changed, 15 insertions(+), 7 deletions(-)
    diff --git a/dogpile/cache/region.py b/dogpile/cache/region.py
    index 7085289..e060407 100644
    --- a/dogpile/cache/region.py
    +++ b/dogpile/cache/region.py
    @@ -982,10 +982,11 @@ class CacheRegion(object):
             def decorator(fn):
                 if to_str is compat.string_type:
                     # backwards compatible
    -                key_generator = self.function_key_generator(namespace, fn)
    +                key_generator0, key_generator = \
    +                        self.function_key_generator(namespace, fn)
    -                key_generator = self.function_key_generator(namespace, fn,
    -                                    to_str=to_str)
    +                key_generator0, key_generator = \
    +                        self.function_key_generator(namespace, fn, to_str=to_str)
                 def decorate(*arg, **kw):
                     key = key_generator(*arg, **kw)
    @@ -998,15 +999,15 @@ class CacheRegion(object):
                 def invalidate(*arg, **kw):
    -                key = key_generator(*arg, **kw)
    +                key = key_generator0(*arg, **kw)
                 def set_(value, *arg, **kw):
    -                key = key_generator(*arg, **kw)
    +                key = key_generator0(*arg, **kw)
                     self.set(key, value)
                 def refresh(*arg, **kw):
    -                key = key_generator(*arg, **kw)
    +                key = key_generator0(*arg, **kw)
                     value = fn(*arg, **kw)
                     self.set(key, value)
                     return value
    diff --git a/dogpile/cache/util.py b/dogpile/cache/util.py
    index 8f6f3ff..41f6d08 100644
    --- a/dogpile/cache/util.py
    +++ b/dogpile/cache/util.py
    @@ -72,6 +72,13 @@ def function_key_generator(namespace, fn, to_str=compat.string_type):
         args = inspect.getargspec(fn)
         has_self = args[0] and args[0][0] in ('self', 'cls')
    +    def generate_key0(*args, **kw):
    +        if kw:
    +            raise ValueError(
    +                    "dogpile.cache's default key creation "
    +                    "function does not accept keyword arguments.")
    +        return namespace + "|" + " ".join(map(to_str, args))
         def generate_key(*args, **kw):
             if kw:
                 raise ValueError(
    @@ -81,7 +88,7 @@ def function_key_generator(namespace, fn, to_str=compat.string_type):
                 args = args[1:]
             return namespace + "|" + " ".join(map(to_str, args))
    -    return generate_key
    +    return generate_key0, generate_key
     def function_multi_key_generator(namespace, fn, to_str=compat.string_type):
  7. Michael Bayer repo owner

    yeah the issue is how to handle a user-defined key generation function, particularly one that is dependent on actually handling the "self" or "cls" argument. Also the region only accepts a single function as the key generator.

    im also not even sure i find the "region-level key generation function" to be very useful, the other day i really wanted one that works at the per-function level, though there seemed to be other awkwardnesses with that. so in general, revisiting how we generate keys from these functions and allow customization is something that needs to happen.

  8. Log in to comment