Skip to content

Fixed #20945 -- Allow cache tag to use a specific cache #1490

Closed
wants to merge 1 commit into from

2 participants

@funkybob

This PR adds two features:
1) Have cache tag use the cache called "template_fragments" if it exists, otherwise use "default".

2) Allow specifying which cach to use on a specific cache tag:
{% cache ....... using="cachename" %}

https://code.djangoproject.com/ticket/20945

@funkybob

Just rebased against master and squashed.

@timgraham timgraham commented on an outdated diff Sep 5, 2013
django/templatetags/cache.py
return CacheNode(nodelist,
parser.compile_filter(tokens[1]),
tokens[2], # fragment_name can't be a variable.
- [parser.compile_filter(token) for token in tokens[3:]])
+ [parser.compile_filter(token) for token in tokens[3:]],
+ cache_name
@timgraham
Django member
timgraham added a note Sep 5, 2013

I like to include a trailing comma in cases like this so if more args are later edited it doesn't require editing this line again (keeps git blame cleaner)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
docs/releases/1.7.txt
@@ -265,6 +265,12 @@ Templates
* It is now possible to :ttag:`include` templates recursively.
+* The :ttag:`cache` will now try to use the cache called "template_fragments"
+ if it exists, and fall back to using "default" otherwise.
@timgraham
Django member
timgraham added a note Sep 5, 2013

no comma (only include one if the two clauses joined by "and" could be independent sentences). I'd also say "the default cache" instead of "default"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham and 1 other commented on an outdated diff Sep 5, 2013
django/templatetags/cache.py
register = Library()
+try:
+ default_cache = get_cache('template_fragments')
+except InvalidCacheBackendError:
+ from django.core.cache import cache as default_cache
+
+
+class CacheDict(defaultdict):
@timgraham
Django member
timgraham added a note Sep 5, 2013

is this still needed if the fix for #21012 is committed?

@funkybob
funkybob added a note Sep 6, 2013

No. Once any solution is applied for the caching issue #21012 addresses [Florian rightly rejects that solution as under-developed] then this whole wad of work will not be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
docs/topics/cache.txt
@@ -637,6 +637,18 @@ equivalent:
This feature is useful in avoiding repetition in templates. You can set the
timeout in a variable, in one place, and just reuse that value.
+By default, the cache tag will try to use the cache called
@timgraham
Django member
timgraham added a note Sep 5, 2013

need versionadded here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
docs/topics/cache.txt
@@ -637,6 +637,18 @@ equivalent:
This feature is useful in avoiding repetition in templates. You can set the
timeout in a variable, in one place, and just reuse that value.
+By default, the cache tag will try to use the cache called
+"template_fragments". If no such cache exists, it will fall back to using
@timgraham
Django member
timgraham added a note Sep 5, 2013

only one space between sentences (barbaric, I know...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
docs/topics/cache.txt
@@ -637,6 +637,18 @@ equivalent:
This feature is useful in avoiding repetition in templates. You can set the
timeout in a variable, in one place, and just reuse that value.
+By default, the cache tag will try to use the cache called
+"template_fragments". If no such cache exists, it will fall back to using
+"default". You can optionally select an alternate cache backend to use. This
+is controlled with the `using` keyword argument, which must be the last
@timgraham
Django member
timgraham added a note Sep 5, 2013

double `` around using

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
tests/template_tests/tests.py
@@ -476,6 +476,66 @@ def test_cache_regression_20130(self):
cachenode = t.nodelist[1]
self.assertEqual(cachenode.fragment_name, 'regression_20130')
+ @override_settings(CACHES={
+ 'default': {
+ 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+ 'LOCATION': 'default'
@timgraham
Django member
timgraham added a note Sep 5, 2013

like trailing commas in the last item in a dictionary as stated above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on the diff Sep 5, 2013
tests/template_tests/tests.py
+ {% cache 1 test1 using="other" %}more stuff{% endcache %}
+ ''')
+ ctx = Context()
+ output = tmpl.render(ctx)
+ # Now verify there's only one instance of 'other'
+ self.assertEqual(1, DummyCacheBackend.count)
+
+ def test_cache_missing_backend(self):
+ '''
+ When a cache that doesn't exist is specified, the cache tag will
+ raise a TemplateSyntaxError
+ '''
+ t = Template('{% load cache %}{% cache 1 backend using="unknown" %}bar{% endcache %}')
+
+ ctx = Context()
+ with self.assertRaises(TemplateSyntaxError):
@timgraham
Django member
timgraham added a note Sep 5, 2013

there's also six.assertRaisesRegex which can be useful for checking the message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
tests/template_tests/tests.py
@@ -1886,3 +1945,20 @@ def test_stack_size(self):
# The stack should now contain 3 items:
# [builtins, supplied context, context processor]
self.assertEqual(len(ctx.dicts), 3)
+
+
+class DummyCacheBackend(object):
+ """
+ A dummy cache backend that counts how many times it is instanciated.
+ """
+ count = 0
+ def __init__(self, *args, **kwargs):
+ self.__class__.count += 1
+ self.values = {}
+ def get(self, key, *args, **kwargs):
@timgraham
Django member
timgraham added a note Sep 5, 2013

please include newlines between methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham and 1 other commented on an outdated diff Sep 5, 2013
tests/template_tests/tests.py
@@ -1886,3 +1945,20 @@ def test_stack_size(self):
# The stack should now contain 3 items:
# [builtins, supplied context, context processor]
self.assertEqual(len(ctx.dicts), 3)
+
+
+class DummyCacheBackend(object):
@timgraham
Django member
timgraham added a note Sep 5, 2013

at the risk of being a bit verbose, could we call it something like DummyCounterCacheBackend so it's not confused with the DummyCache provided by Django?

@funkybob
funkybob added a note Sep 6, 2013

As mentioned above, all of this cache caching should be removed once a proper caching solution lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham timgraham commented on an outdated diff Sep 5, 2013
tests/template_tests/tests.py
@@ -1886,3 +1945,20 @@ def test_stack_size(self):
# The stack should now contain 3 items:
# [builtins, supplied context, context processor]
self.assertEqual(len(ctx.dicts), 3)
+
+
+class DummyCacheBackend(object):
+ """
+ A dummy cache backend that counts how many times it is instanciated.
@timgraham
Django member
timgraham added a note Sep 5, 2013

spelling: instantiated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@timgraham
Django member

merged in 8688f03.

@timgraham timgraham closed this Oct 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.