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 , 9 years ago
comment:2 by , 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 , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Some discussion on the DevelopersMailingList may be useful to decide on how to proceed.
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?