[Django] #25501: Filebased cache should use the highest pickling protocol
Django
-------------------------------------+--------------------
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+--------------------
I came across this bug when caching `psycopg2._range.NumericRange` objects
in [https://github.com/BertrandBordage/django-cachalot django-cachalot]
(in order to add Django 1.8 support to it). [https://travis-
ci.org/BertrandBordage/django-cachalot/builds/83412824 My test suite was
failing] only on Python 2 and with filebased (and of course, Django 1.8,
because `django.contrib.postgres` didn’t exist before). This error was
thrown:
{{{
TypeError: a class that defines __slots__ without defining __getstate__
cannot be pickled
}}}
This is because, unlike
[https://github.com/django/django/blob/0ed7d155635da9f79d4dd67e4889087d3673c6da/django/core/cache/backends/locmem.py#L75
locmem] or
[https://github.com/django/django/blob/e2a652fac1ce9af51e2dfdfb4e26a1c94cf3189c/django/core/cache/backends/db.py#L114
db],
[https://github.com/django/django/blob/0ed7d155635da9f79d4dd67e4889087d3673c6da/django/core/cache/backends/filebased.py#L58
filebased uses the default pickling protocol] and not the highest
available. Since we are not trying to achieve read compatibility with old
Python versions, we can use the highest protocol here as well.
This issue occur on master, Django 1.7.10, 1.8.4, & 1.9 alpha 1.
--
Ticket URL: <https://code.djangoproject.com/ticket/25501>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:1>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by dudepare):
* owner: nobody => dudepare
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:2>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by dudepare):
Hi Bertrand,
I am a new to contributing to Django. Do you have any steps on how to
reproduce this?
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:3>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by BertrandBordage):
Hi Andrew,
If you need to be able to test it when psycopg2 is not installed, you can
also reproduce this bug by creating a class with the same problem:
{{{#!python
class UnpickableType(object):
__slots__ = ('a',)
}}}
If you try to pickle an instance of this class, you’ll get the same error.
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:4>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/5416 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:5>
Django
Reporter: BertrandBordage | Owner: dudepare
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"48888a1a67db0404e5a31c9ca0349984e496f26f" 48888a1]:
{{{
#!CommitTicketReference repository=""
revision="48888a1a67db0404e5a31c9ca0349984e496f26f"
Fixed #25501 -- Made the file-based cache backend use the highest pickling
protocol.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:6>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by BertrandBordage):
Great job, thanks :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:7>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by dudepare):
Thanks for your help Bertrand.
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:8>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by jaap3):
I've commented on the commit, but I'll do it here as well:
The original intent has always been to use the highest protocol. From
help(pickle.dumps):
Specifying a negative protocol version selects the highest protocol
version supported.
So this line:
f.write(zlib.compress(pickle.dumps(value), -1))
Was supposed to be:
f.write(zlib.compress(pickle.dumps(value, -1)))
Calling zlib.compress with a compression level of -1 seems to fall back to
the default level of 6.
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:9>
Django
Reporter: BertrandBordage | Owner: dudepare
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1aba0e4c68111c99e15d9a4f4d7b4a0ec3c5da01" 1aba0e4c]:
{{{
#!CommitTicketReference repository=""
revision="1aba0e4c68111c99e15d9a4f4d7b4a0ec3c5da01"
Refs #25501 -- Fixed a typo in django/core/cache/backends/filebased.py
The original intent in refs #20536 was to use the highest protocol.
Calling zlib.compress() with a compression level of -1 seems to
fall back to the default level of 6.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25501#comment:10>