[Django] #20289: Unpickling of dynamic model classes broken in 1.5
Django
----------------------------------------------+--------------------
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
The commit 146aff3bac974e56ec8cb597c2720d1cd4f77b26 broke unpickling of
dynamically created models. This was reported in django-users thread
http://groups.google.com/group/django-
users/browse_thread/thread/1eee4a4d5098bfc5.
The problem is that deferred classes aren't only possible source of
dynamically created class in Django. ManyToMany through class are
dynamically created and in addition dynamic classes are possible in user
code. My understanding is that user code dynamic classes should be
supported, too.
I am not sure how to trigger this bug using standard Django models and
fields. The django-users thread mentions this:
{{{
So for example, I have an instance of a Student, which has a
ForeignKey to Book, which in turn has a ManyToMany to Author.
If I try and cache my Student instance, I get that pickling
error.
}}}
but I haven't tested this.
I think resurrecting the simple_class_factory path of code instead of
using `super.__reduce__` for the non deferred case is the right fix. Of
course, the code comments need fixing, too, so just reverting the commit
isn't a good idea.
Note that I haven't done any actual testing (no Django available on this
machine...), so I will leave this unreviewed until I (or somebody else)
will reproduce this using Django 1.5.
--
Ticket URL: <https://code.djangoproject.com/ticket/20289>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* cc: charettes (added)
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:1>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by MeirKriheli):
Any news regarding that ? It breaks caching, had to be disabled for now,
which takes it's toll from sites.
Thanks
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:2>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
I think I have something that fixes this, see:
https://github.com/akaariai/django/commit/6730c97f96fe1377fe81a8e4d089257bd36e3df6
However those test cases do not pass on 1.4 (see
https://github.com/akaariai/django/tree/ticket_20289_14). So, I would like
to see a reproducible regression between 1.4 and 1.5 so that it is sure we
got the regression fixed.
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:3>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by andrewgodwin):
Yes, I can't get anything to pass on 1.4 either, either using a container
or direct type() calls with models and various combinations of `app_label`
and `__module__`.
Provided the tests do now pass on trunk, though, I'm reasonably happy to
call this ticket fixed, though perhaps we should add something like this
to the test too (as it's what the mailing list says was happening):
{{{
meta = type("Meta", (object, ), {"app_label": "queryset_pickle"})
original = type("MyTestModel", (models.Model, ), {"Meta": meta,
"__module__": Group.__module__, ...fields...})
dumped = pickle.dumps(original)
reloaded = pickle.loads(dumped)
self.assertEqual(original, reloaded)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:4>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* stage: Accepted => Ready for checkin
Comment:
I guess we can just commit this one with the added test? I'll mark this
RFC, the test of comment:4 should be likely added, but that is easy enough
to do when committing...
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:5>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.5
Severity: Release blocker | Triage Stage: Ready for
Keywords: | checkin
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Anssi Kääriäinen <akaariai@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"5459795ef224c5c81461c06a95d38390ee91f014"]:
{{{
#!CommitTicketReference repository=""
revision="5459795ef224c5c81461c06a95d38390ee91f014"
Fixed #20289 -- pickling of dynamic models
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:6>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by akaariai):
* status: closed => new
* resolution: fixed =>
* stage: Ready for checkin => Accepted
Comment:
I don't feel comfortable backpatching this unless absolutely needed. The
report was that things break when upgrading from 1.4 to 1.5, but I haven't
been able to found any case where we have a model pickling regression.
Could it be that the breakage isn't actually due to changes in model
pickling, but trying to load 1.4 pickled data with 1.5 unpickle. We don't
guarantee pickling compatibility between versions, so if this is the case
then we shouldn't backpatch (not that backpatching even helps here).
I will reopen this as 1.5 release blocker, lets see what we need to do to
this before 1.5.2.
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:7>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.5
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* status: new => closed
* resolution: => fixed
Comment:
I think it is time to close this one. This ticket is still missing an
example of the claimed regression. I believe the problem is actually in
trying to use model pickling across versions. In my understanding that
isn't officially supported.
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:8>
Django
Reporter: akaariai | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.5
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by almostabc):
I know this bug has been closed, but I'm running into the same thing and
am 99% sure it isn't a cross-version pickling issue. I'm working on a
simple test case that demonstrates this problem and will update this bug
when it is ready.
--
Ticket URL: <https://code.djangoproject.com/ticket/20289#comment:9>