[Django] #20584: Django's Memcached backend get_many() doesn't handle generators
Django
-------------------------------------+--------------------
Reporter: guyon.moree@… | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+--------------------
When the "keys" parameter to get_many() is a generator, the values will be
lost in the zip function.
https://github.com/django/django/blob/master/django/core/cache/backends/memcached.py#L93
Here's a simplified code example:
{{{
def make_key(k):
return k
user_ids = (11387, 1304318)
keys = ('user_%d' % x for x in user_ids)
new_keys = map(lambda x: make_key(x), keys)
m = dict(zip(new_keys, keys))
assert( m == {} )
}}}
I believe this is related to this zip() behaviour:
http://stackoverflow.com/questions/11210300/why-does-zip-drop-the-values-
of-my-generator
I encountered this bug when upgrading from django 1.3 to django 1.5.1
--
Ticket URL: <https://code.djangoproject.com/ticket/20584>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: guyon.moree@… | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: 1.5
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* status: new => closed
* needs_better_patch: => 0
* resolution: => needsinfo
* needs_tests: => 0
* needs_docs: => 0
Comment:
Hi,
The code example shows the expected behavior of a generator: once you
iterate over it, it's empty.
Can you describe the actual bug you're encountering in your django
application?
The commit that added the line you linked to [1] was already included in
1.3 so your problem must be elsewhere.
I'm going to mark this as `needsinfo`. Please re-open the ticket with an
example on how you trigger the issue from django.
Thanks.
[1]
https://github.com/django/django/commit/99d247f4cb0c22d19a4482a72a7a93584a5189da
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:1>
Django
Reporter: guyon.moree@… | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: 1.5
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anonymous):
Ah, I misunderstood the problem myself, makes sense now. It worked in a
previous version because the generator was not exhausted there.
thanks a lot, makes sense now
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: guyon.moree@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Christian Barcenas):
* status: closed => new
* version: 1.5 => master
* has_patch: 0 => 1
* resolution: needsinfo =>
* stage: Unreviewed => Ready for checkin
Old description:
> When the "keys" parameter to get_many() is a generator, the values will
> be lost in the zip function.
>
> https://github.com/django/django/blob/master/django/core/cache/backends/memcached.py#L93
>
> Here's a simplified code example:
>
> {{{
> def make_key(k):
> return k
>
> user_ids = (11387, 1304318)
>
> keys = ('user_%d' % x for x in user_ids)
>
> new_keys = map(lambda x: make_key(x), keys)
>
> m = dict(zip(new_keys, keys))
>
> assert( m == {} )
> }}}
>
> I believe this is related to this zip() behaviour:
> http://stackoverflow.com/questions/11210300/why-does-zip-drop-the-values-
> of-my-generator
>
> I encountered this bug when upgrading from django 1.3 to django 1.5.1
New description:
When the `keys` parameter to `get_many()` is an iterator, its values will
be consumed in a list comprehension, but then later the already-consumed
iterator is passed to `zip`.
https://github.com/django/django/blob/master/django/core/cache/backends/memcached.py#L90
This causes a very confusing `ValueError` which is raised when the cache
backend attempts to map lower-level Memcache-backend cache keys back to
higher-level Djanco cache keys.
{{{
Traceback (most recent call last):
File "[snip]/django/tests/cache/tests.py", line 1345, in
test_get_many_accepts_iterator
values = cache.get_many(iter(['fizz']))
File "[snip]/django/lib/python3.6/site-
packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
line 91, in get_many
return {m[k]: v for k, v in ret.items()}
File "[snip]/django/lib/python3.6/site-
packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
line 91, in <dictcomp>
return {m[k]: v for k, v in ret.items()}
KeyError: ':1:fizz'
}}}
--
Comment:
Re-opening this ticket, and updating the description with more details.
The Memcache backend's `get_many()` function definitely fails when the key
inputs are iterables which can be consumed only once, such as generators.
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:3>
Django
-------------------------------------+-------------------------------------
| Barcenas
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Christian Barcenas):
* owner: nobody => Christian Barcenas
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: guyon.moree@… | Owner: Christian
| Barcenas
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Christian Barcenas):
A patch is ready for review on Github:
https://github.com/django/django/pull/10136
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: guyon.moree@… | Owner: Christian
| Barcenas
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Christian Barcenas:
Old description:
> When the `keys` parameter to `get_many()` is an iterator, its values will
> be consumed in a list comprehension, but then later the already-consumed
> iterator is passed to `zip`.
>
> https://github.com/django/django/blob/master/django/core/cache/backends/memcached.py#L90
>
> This causes a very confusing `ValueError` which is raised when the cache
> backend attempts to map lower-level Memcache-backend cache keys back to
> higher-level Djanco cache keys.
>
> {{{
> Traceback (most recent call last):
> File "[snip]/django/tests/cache/tests.py", line 1345, in
> test_get_many_accepts_iterator
> values = cache.get_many(iter(['fizz']))
> File "[snip]/django/lib/python3.6/site-
> packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
> line 91, in get_many
> return {m[k]: v for k, v in ret.items()}
> File "[snip]/django/lib/python3.6/site-
> packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
> line 91, in <dictcomp>
> return {m[k]: v for k, v in ret.items()}
> KeyError: ':1:fizz'
> }}}
New description:
When the `keys` parameter to `get_many()` is an iterator, its values will
be consumed in a list comprehension. However, later the already-consumed
iterator is then passed to `zip`.
https://github.com/django/django/blob/master/django/core/cache/backends/memcached.py#L90
This causes a very confusing `KeyError` which is raised when the cache
backend attempts to map lower-level Memcache-backend cache keys back to
higher-level Djanco cache keys.
{{{
Traceback (most recent call last):
File "[snip]/django/tests/cache/tests.py", line 1345, in
test_get_many_accepts_iterator
values = cache.get_many(iter(['fizz']))
File "[snip]/django/lib/python3.6/site-
packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
line 91, in get_many
return {m[k]: v for k, v in ret.items()}
File "[snip]/django/lib/python3.6/site-
packages/Django-2.2.dev20180706090656-py3.6.egg/django/core/cache/backends/memcached.py",
line 91, in <dictcomp>
return {m[k]: v for k, v in ret.items()}
KeyError: ':1:fizz'
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: guyon.moree@… | Owner: Christian
| Barcenas
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: guyon.moree@… | Owner: Christian
| Barcenas
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c9c6c166506bec59c57d4e3389e7ccd552e47ffc" c9c6c16]:
{{{
#!CommitTicketReference repository=""
revision="c9c6c166506bec59c57d4e3389e7ccd552e47ffc"
Fixed #20584 -- Fixed memcached's get_many() with single-use iterators.
Thanks Guyon Morée for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20584#comment:8>