Opened 9 years ago

Last modified 9 years ago

#26619 new Bug

BaseCache incr method will reset the timeout

Reported by: Esteban C Borsani Owned by: nobody
Component: Core (Cache system) Version: 1.8
Severity: Normal Keywords: cache, incr, DatabaseCache, BaseCache
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

Description

BaseCache.incr() resets the timeout to the default timeout. DatabaseCache and FileBasedCache are affected by this, since they do not override the method. I'm not sure what the memcache behaviour is.

To reproduce this, set the default cache backend to django.core.cache.backends.db.DatabaseCache and call incr() on the cache. Although, the bug is easy to spot since .set(...) is being called without passing a timeout.

I'm assuming this is a bug since LocMemCache doesn't reset the timeout, the docs don't mention anything about it and it seems like a surprising behaviour to me.

Django 1.8 and 1.9 are affected by this.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (3)

comment:1 by Tim Graham, 9 years ago

I guess a possible resolution would be to add a timeout argument to those methods? I don't think we can increment/ decrement them while keeping the current timeout?

comment:2 by Esteban C Borsani, 9 years ago

Honestly, I'm not sure. Your proposal is consistent with set(). However, memcached and Redis don't set a timeout when incr/decr. I don't know of a in-memory cache that does that.

The database backend could just update instead of doing a get/set, so it could keep the timeout (and also be atomic), not sure why it was not implemented that way. The filecache backend can also leave the timeout untouched, AFAICT.

If I were to design the cache system from scratch I'd just raise NotImplementedError within the incr/decr of the base backend. But at this point, I don't know how many third-party libs that would break.

comment:3 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

Some discussion on the DevelopersMailingList may be useful to decide on how to proceed.

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