Opened 7 years ago

Closed 4 years ago

#6988 closed (wontfix)

Inconsistent CACHE_MIDDLEWARE_SECONDS = 0 meaning across cache backends

Reported by: Simon Law <simon@…> Owned by: nobody
Component: Core (Cache system) Version: master
Severity: Keywords: CacheMiddleware memcached CACHE_MIDDLEWARE_SECONDS
Cc: eric@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

According to http://www.djangoproject.com/documentation/cache/#the-per-site-cache, settings.CACHE_MIDDLEWARE_SECONDS is "The number of seconds each page should be cached."

However, memcached takes a 0 second timeout to mean "cache forever" instead of "do not cache".

This patch enforces the Django meaning in django.core.cache.backends.memcached

Attachments (3)

django-memcached.patch (1.4 KB) - added by Simon Law <simon@…> 7 years ago.
django-memcached.2.patch (1.3 KB) - added by Simon Law <simon@…> 7 years ago.
django-memcached.3.patch (1.4 KB) - added by Simon Law <simon@…> 7 years ago.
memcached.py has the same semantics as other django.core.cache.backends

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Simon Law <simon@…>

Changed 7 years ago by Simon Law <simon@…>

comment:1 Changed 7 years ago by Simon Law <simon@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

The second patch is far more correct.

comment:2 Changed 7 years ago by ubernostrum

I guess I don't understand why someone would call cache.add() or cache.set() with something that's not meant to be cached...

comment:3 Changed 7 years ago by PhiR

  • Resolution set to wontfix
  • Status changed from new to closed

It is very common for 0 to mean "infinity" for those kind of settings. If you want to submit a patch to django's documentation to make that explicit, feel free to do so. If you want to disable caching in the settings then change the engine to dummy.

comment:4 Changed 7 years ago by Simon Law <simon@…>

  • Resolution wontfix deleted
  • Status changed from closed to reopened

cache.add() and cache.set() with a timeout of 0 is used when CacheMiddleware is given a 0 second timeout.

You would want it in this case:

from django.conf import settings
from django.views.decorators.cache import cache_page

settings.CACHE_MIDDLEWARE_SECONDS = 300  # Obviously, this is done in settings.py

@cache_page(0)
def never_cache_this(request):
    ...

This works with all CACHE_BACKENDs other than memcached. This means that the semantics are different if you choose locmem:// instead of memcached://.../ which is not something that should be solved by documentation.

If 0 seconds means "cache forever", then I would be happy to prepare a patch to cause the other cache backends to behave the same way. But I believe that a 0 second timeout is valuable, because of CacheMiddleware's interface.

Changed 7 years ago by Simon Law <simon@…>

memcached.py has the same semantics as other django.core.cache.backends

comment:5 Changed 7 years ago by wiswaud

  • Cc eric@… added

comment:6 Changed 7 years ago by telenieko

  • Summary changed from Memcached caches forever when CACHE_MIDDLEWARE_SECONDS = 0 to Inconsistent CACHE_MIDDLEWARE_SECONDS = 0 meaning across cache backends
  • Triage Stage changed from Unreviewed to Accepted

I understand that the issue is not that "0" means infinity, but that 0 means different things depending on the backend you use.
So either 0 means infinity to all, or means "do not cache".

comment:7 Changed 4 years ago by otherjacob

Eligible to be closed due to recent changes/tests no longer requiring 0 as a insta-invaliation. May be worth noting in the documentation the difference, but a fair warning of YMMV should be enough.

comment:8 Changed 4 years ago by otherjacob

  • Resolution set to wontfix
  • Status changed from reopened to closed

And closing to reflect the above.

Note: See TracTickets for help on using tickets.
Back to Top