Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12163 closed (fixed)

Regression: Pickling error saving session with unsaved model instances (*_Deferred_)

Reported by: rfugger Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: pickle, session, model, instance, deferred
Cc: arv@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When attempting to store freshly-created unsaved model instances in a session, I get the following error:

PicklingError: Can't pickle <class 'order.models.OrderItem_Deferred_'>: attribute lookup order.models.OrderItem_Deferred_ failed

The object is an instance of OrderItem. Apparently django instantiates it as !OrderItem_Deferred_, and then pickle can't deal with it?

Using svn revision 11723. Worked fine in 11616.

Attachments (2)

base.py.diff (495 bytes) - added by rfugger 4 years ago.
Set model to _meta.proxy_for_model by default for deferred
12163_test.diff (983 bytes) - added by rfugger 4 years ago.
Test case added to defer_regress tests.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by russellm

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

There have been some recent changes to the infrastructure around pickling of deferred fields - evidently you have found a case that wasn't covered by our test suite.

Can you please provide a complete test case - i.e., a minimal set of models, and the exact sequence of commands that fails.

comment:2 Changed 4 years ago by rfugger

I'll try to work up a test case when I have time. Strangely, when I reverted to r11616, the __reduce__ function was broken (model used before assignment), even though it was definitely working fine three days ago. So maybe I'm doing something wrong, although I don't think my surrounding code has changed since then.

In the meantime, replacing model = self.__class__ with model = self._meta.proxy_for_model near the start of db.models.base.Model.__reduce__ seems to fix the issue for me. I'm not sure if this is a good general solution or not, because I don't really know how deferred fields work.

Changed 4 years ago by rfugger

Set model to _meta.proxy_for_model by default for deferred

comment:3 Changed 4 years ago by rfugger

(Sorry, probably better not to replace model = self.__class__, but just to set model = self._meta.proxy_for_model as a default when self._deferred is True.

comment:4 Changed 4 years ago by rfugger

  • Cc arv@… added

Changed 4 years ago by rfugger

Test case added to defer_regress tests.

comment:5 Changed 4 years ago by AndrewIngram

I'd just like to add that it's revision 11691 that causes this, using 11690 should fix things for now.

comment:6 Changed 4 years ago by rfugger

Thanks Andrew. r11690 does work. I hadn't cleared out my session last time I tried a pre-11691 revision, so pickled *_Deferred_ objects were still hanging around and causing errors.

comment:7 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [11732]) Fixed #12163 -- Corrected the unpickling of non-deferred models. Thanks to rfugger for the report and test case.

comment:8 Changed 4 years ago by russellm

(In [11733]) [1.1.X] Fixed #12163 -- Corrected the unpickling of non-deferred models. Thanks to rfugger for the report and test case.

Backport of r11732 from trunk.

comment:9 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.