Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#8134 closed (fixed)

loaddata refuses to load intermediary model target model

Reported by: rockmhoward@… Owned by: kire
Component: Core (Serialization) Version: master
Severity: Keywords: loaddata intermediary
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I used dumpdata to dump output for a model and its' intermediary model which was defined in the same models.py file. The dumped data (which is in JSON format) looks fine.

Later I tried to use loadata to read in this JSON file, but that failed and the failure was denoted by an AttributeError telling me to use the intermediary model's manager to load the data. Well then I guess that loaddata should do that (or some other workaround put in place to allow loaddata to do its' thing.)

I don't have a trivial example at hand right now, but I will supply one if it is needed.

I am using a very recent trunk (8022) in development. I have used the intermediary model that I have created quite heavily in development mode and it works great. I discovered the problem when moving my app to my production server which is also running a very recent version of trunk.

Attachments (3)

intermediaryfixtures.diff (715 bytes) - added by kire 6 years ago.
Patch proposal
intermediatefixturestest.diff (3.9 KB) - added by kire 6 years ago.
test cases
imtest1.tar.gz (3.4 KB) - added by rock@… 6 years ago.
small app with README to reproduce actual loaddata SNAFU

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by russellm

  • milestone set to 1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I can see why this would happen. m2m intermediate models can't be serialized like normal m2m fields; I'm guessing the solution is that the deserializer needs to consider the 'creates_table' attribute on the m2m field. However, we will need to resolve this before v1.0

comment:2 Changed 6 years ago by kire

  • Owner changed from nobody to kire
  • Status changed from new to assigned

Quickly looked in to it, in my opinion the real mistake lies with the save method of the DeserializedObject. Will commit a pacht within a moment.

Changed 6 years ago by kire

Patch proposal

comment:3 follow-up: Changed 6 years ago by kire

  • Has patch set

I tested the patch on my own site that uses both intermediary and non-intermediary manyToMany fields. All fixtures worked fine.

I looked into the loaddata and deserialization as suggested for the patch but wouldn't it actually be better just not to serialize intermediary ManyToMany fields to begin with?

comment:4 in reply to: ↑ 3 Changed 6 years ago by russellm

  • Component changed from django-admin.py to Serialization

Replying to kire:

I looked into the loaddata and deserialization as suggested for the patch but wouldn't it actually be better just not to serialize intermediary ManyToMany fields to begin with?

Thinking about this some more, you're probably right. Attempting to deserialize m2m_field = [1,2,3] should be an error; I haven't checked, but I suspect that this will be what an m2m field outputs by default, regardless of whether it is an intermediate or not. m2m fields should be skipped in serialization, relying upon the serialization of the intermediate model.

Regardless of the solution, the patch needs a test case before it will be committed. Tests are not optional :-)

Changed 6 years ago by kire

test cases

comment:5 Changed 6 years ago by kire

Okay, since cmopletely getting m2m out of the serialization process was a little more work I decided to provide tests for my original patch first. I'm not the guy to triage here, but I suggest you apply this patch so it works and create a new ticket to recode the serialization system. Maybe I'll accept that one also, but at least there's a fix for now ;-).

Changed 6 years ago by rock@…

small app with README to reproduce actual loaddata SNAFU

comment:6 Changed 6 years ago by russellm

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

(In [8321]) Fixed #8134 -- Corrected serialization of m2m fields with intermediate models. Thanks to Rock Howard for the report, and kire for the test case.

comment:7 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 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.