#33061: ValueError not raised by some cache backends for incr() / decr() with
negative delta when a key is missing
-------------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Sondre Lillebø Gundersen):
Replying to [comment:4 Chris Jerdonek]:
> > Why do you think that we should raise an error in this case?
>
> It's not that an error should be raised from nothing. It's that
`ValueError` should be raised instead of `LibraryValueNotFoundException`
(or returning `None`) when a key is missing. If you look at the links to
the code I provided, the `delta < 0` code path isn't inside the try-except
block a line later where that conversion takes place for the `delta >= 0`
case.
Hi!
I was looking at trying to implement this ticket, but I'm confused by one
thing (perhaps I'm missing something):
*Won't* the `delta < 0` code path for `incr` raise a `ValueError` even
though it's not in the `incr` try/except block? Since it calls `decr` and
that method has its own mirrored logic, it seems like you would end up
with a `ValueError` either way, wouldn't you?
{{{#!python
def incr(self, key, delta=1, version=None):
key = self.make_key(key, version=version)
# memcached doesn't support a negative delta
if delta < 0:
return self._cache.decr(key, -delta)
try:
val = self._cache.incr(key, delta)
# python-memcache responds to incr on non-existent keys by
# raising a ValueError, pylibmc by raising a pylibmc.NotFound
# and Cmemcache returns None. In all cases,
# we should raise a ValueError though.
except self.LibraryValueNotFoundException:
val = None
if val is None:
raise ValueError("Key '%s' not found" % key)
return val
def decr(self, key, delta=1, version=None):
key = self.make_key(key, version=version)
# memcached doesn't support a negative delta
if delta < 0:
return self._cache.incr(key, -delta)
try:
val = self._cache.decr(key, delta)
# python-memcache responds to incr on non-existent keys by
# raising a ValueError, pylibmc by raising a pylibmc.NotFound
# and Cmemcache returns None. In all cases,
# we should raise a ValueError though.
except self.LibraryValueNotFoundException:
val = None
if val is None:
raise ValueError("Key '%s' not found" % key)
return val
}}}
The missing key would not be raised from `incr` but from `decr`, right?
Perhaps I'm missing the point 🙂
--
Ticket URL: <https://code.djangoproject.com/ticket/33061#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.e567e5015d2120f004282df8dffe218d%40djangoproject.com.
