hwms / django-command-extensions

Automatically exported from code.google.com/p/django-command-extensions
MIT License
0 stars 0 forks source link

dumpscript handles generic relations poorly #43

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Dumpscript handles generic relations by dumping out the values of the
contenttype and object_id fields directly.  Since objects created by
running a dumped script may well have a different autonumber PK, this means
that in many cases generic relations will be pointing to the wrong object
after running a dumped script.

The robust solution, it would seem, would be for a dumped script to assign
the proper object directly to the GenericRelation attribute rather than to
the contenttype and object_id fields.

Original issue reported on code.google.com by carl.j.meyer on 19 Aug 2008 at 5:15

GoogleCodeExporter commented 9 years ago
Here is a patch/proof of concept. It relies on Model._meta.virtual_fields, which
might not exist in pre-1.0 django versions.

GenericForeignKey fields are linked using the object, only the virtual field is 
set,
the db fields are ignored. My simple test suite works, but I'm not sure how this
would go with circular ContentType references.

Original comment by e.willha...@gmail.com on 19 Sep 2008 at 7:20

Attachments:

GoogleCodeExporter commented 9 years ago
Nice patch!  I haven't tested with circular refs, but I do have a case (the new
contrib.comments) where the ct_field on a generic relation uses related_name, 
so the
reverse-relation pseudo-field on ContentType has an unexpected name.  I've 
updated
your patch to handle that case.

Original comment by carl.j.meyer on 20 Sep 2008 at 11:04

Attachments:

GoogleCodeExporter commented 9 years ago
If you sign off that your tests all still work with this patch, I'll go ahead 
and
commit it.

Original comment by carl.j.meyer on 20 Sep 2008 at 11:06

GoogleCodeExporter commented 9 years ago
Tests are fine, I cleaned up some old (commented out) code leftover from a 
refactor
and added some more comments.

Original comment by e.willha...@gmail.com on 21 Sep 2008 at 3:37

Attachments:

GoogleCodeExporter commented 9 years ago
Hmm, need to test importing some more before this gets committed.  Just tried
importing one of these new-style dumps for the first time and got:
"sqlite3.IntegrityError: votes.content_type_id may not be NULL".  Haven't 
looked at
it in detail, but I guess the GFK object can't always be assigned right away 
because
of ordering issues?  I wonder if we'll have to assign some temporary placeholder
value for the initial save?

Original comment by carl.j.meyer on 21 Sep 2008 at 3:46

GoogleCodeExporter commented 9 years ago
And I'm also seeing that some regular ForeignKeys are missing entirely from the
dumped script with this patch.

Original comment by carl.j.meyer on 21 Sep 2008 at 3:53

GoogleCodeExporter commented 9 years ago
Sorry, regular FK field isn't missing entirely, but it's delayed, which doesn't 
work
if it's a required field.  I may have something circular going on in this 
project now
that GFKs are order-dependent too, I'll have to take a closer look.

Original comment by carl.j.meyer on 21 Sep 2008 at 3:58

GoogleCodeExporter commented 9 years ago
I was thinking that placeholders might be necessary if circular references 
become a
problem. I'll go write some more tests that make things a little more difficult 
for
dumpscript and try out some ways of getting around this. It's probably a good 
idea to
turn my semi-automated tests into completely automated (regression) tests.

Original comment by e.willha...@gmail.com on 21 Sep 2008 at 4:01

GoogleCodeExporter commented 9 years ago
Would it be possible to push issue #35 along? Doing this will help make automate
tests a little simple, by testing each given model one at a time.

Original comment by e.willha...@gmail.com on 21 Sep 2008 at 4:11

GoogleCodeExporter commented 9 years ago
issue #35 is now solved. How can that help us move forward on this?

Original comment by griff.r...@gmail.com on 15 Nov 2008 at 6:06

GoogleCodeExporter commented 9 years ago
I'm studying for my law exams this week, so I might not be able to do much until
afterwards.

I asked for issue #35 so that this ticket's development could proceed a little 
more
"test driven". Here is a models.py file that I have been using to 
semi-automatically
test dumpscript. With #35, individual models could be tested, rather than the 
whole
database, so each test can be done in isolation and tests wouldn't need to be 
updated
everything something is done differently. should django_commands have a test
directory somewhere for these things?

eg: django-command-extensions/test/dumpscript/testapp/models.py

Original comment by e.willha...@gmail.com on 15 Nov 2008 at 6:30

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by e.willha...@gmail.com on 15 Mar 2009 at 10:31