[Django] #24607: dumpdata (with natural keys) / loaddata failure for model inheriting from User
Django
User
-------------------------------+--------------------
Reporter: denys-duchier | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I am using Django1.8. The exact error message is:
{{{
ValueError: Problem installing fixture
'/home/denys/src/GSM/tmp/bug/person.json': "<Person: >" needs to have a
value for field "user" before this many-to-many relationship can be used.
}}}
I have created a very small example to reproduce the problem; it is
available on github at:
`https://github.com/denys-duchier/django-loaddata-bug.git`
to create the database, migrate, populate the database, then `dumpdata`,
execute:
`step1.bash`
to recreate the database, migrate, and attempt the `loaddata`, execute:
`step2.bash`
that is when the error occurs.
--
Ticket URL: <https://code.djangoproject.com/ticket/24607>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* component: Uncategorized => Core (Serialization)
* needs_tests: => 0
* needs_docs: => 0
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
The issue seems to be caused by the fact that `--natural-primary` causes
the `Person`'s 'id' to be omitted in the fixture dump (since
`AbstractBaseUser` defines `natural_key()`), but without it, there's no
way to connect `Person` instances to the corresponding `User`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:1>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by denys-duchier):
perhaps the serialization of `Person` should include a field:
{{{
"user_ptr": {"username": "sigmund"}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:2>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by denys-duchier):
I am thinking that when we have `use_natural_foreign_keys` and the `pk` is
a relation, we want to recurse.
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:3>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by denys-duchier):
I have created a PR that takes a first stab at this problem:
https://github.com/django/django/pull/4473
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:4>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by denys-duchier):
new PR against master:
https://github.com/django/django/pull/4477
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:5>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:6>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Waiting for a response from the reporter on the pull request, but it seems
to me the issue is that `use_natural_primary_keys` is not compatible with
serializing multi-table inheritance models since the child models require
the primary key to be linked to their parent. I'm not sure that adding a
lot of complexity to essential "undo" the option for the affected models
is worth it. Could we simply document the limitation?
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:7>
Django
User
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by levic):
As it stands right now, `use_natural_primary_keys` doesn't work with
multi-table inheritance with or without `use_natural_foreign_keys` -- the
patch at least allows it to work if both `use_natural_primary_keys` and
`use_natural_foreign_keys` are enabled.
The issue only arises when `use_natural_primary_keys` is set and
`use_natural_foreign_keys` is not. In this case possible options are:
1. `use_natural_primary_keys` implies `use_natural_foreign_keys`
1. Always
1. If there are any multi-table inheritance models defined that have a
natural key
1. Only for references to models that are involved in any multi-table
inheritance
- May be a little surprising if you add an inherited model and it
changes the dump for the parent model changes
1. `use_natural_primary_keys` does not imply `use_natural_foreign_keys`
- dumps may break if load/dump order is not identical as the real PKs may
not match (ie is likely to break)
- If there are any multi-table inheritance models with natural keys then
a warning can be emitted
1. `use_natural_primary_keys` is ignored for models that are involved in
any multi-table inheritance and have a natural_key defined
- May be a little surprising if you add an inherited model and it changes
the dump for the parent model changes
Option 2 involves the least change and with the warning follows the
principle of least surprise. It is an improvement over the current
situation and is as backwards-compatible as possible.
If the patch was bought up to date with the latest django version, and it
was changed to use option 2 (so `use_natural_primary_keys` did not turn on
`use_natural_foreign_keys` but gave a warning if a likely problem was
detected) then would this be acceptable?
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:8>
Django
keys
--------------------------------------+------------------------------------
Reporter: denys-duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by charettes):
* version: 1.8 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:9>
Django
keys
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by jpmelos):
New PR on this:
https://github.com/django/django/pull/7231
Context:
https://groups.google.com/d/msg/django-developers/HdfvARojZeo/e37rpYI_FAAJ
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:10>
Django
keys
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:11>
Django
keys
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:11>
Django
keys
--------------------------------------+------------------------------------
Reporter: Denys Duchier | Owner: nobody
Component: Core (Serialization) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a" 74a575eb]:
{{{
#!CommitTicketReference repository=""
revision="74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a"
Fixed #24607 -- Serialized natural keys in multi-table inheritance models.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:12>
Django
keys
--------------------------------------+------------------------------------
Reporter: Denys Duchier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"0595bca221825c0c6bd572a32f3bf9eff7069328" 0595bca]:
{{{
#!CommitTicketReference repository=""
revision="0595bca221825c0c6bd572a32f3bf9eff7069328"
Fixed #27742 -- Reverted "Fixed #24607 -- Serialized natural keys in
multi-table inheritance models."
This reverts commit 74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a as it causes
unexpected migrations and doesn't seem to be the best solution.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:13>
Django
keys
--------------------------------------+------------------------------------
Reporter: Denys Duchier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"eeb28f47512c5ff969d014ba35b9e955de1583ce" eeb28f47]:
{{{
#!CommitTicketReference repository=""
revision="eeb28f47512c5ff969d014ba35b9e955de1583ce"
[1.11.x] Fixed #27742 -- Reverted "Fixed #24607 -- Serialized natural keys
in multi-table inheritance models."
This reverts commit 74a575eb7296fb04e1fc2bd4e3f68dee3c66ee0a as it causes
unexpected migrations and doesn't seem to be the best solution.
Backport of 0595bca221825c0c6bd572a32f3bf9eff7069328 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:14>
Django
keys
--------------------------------------+------------------------------------
Reporter: Denys Duchier | Owner: nobody
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: closed => new
* has_patch: 1 => 0
* resolution: fixed =>
Comment:
Reopening per discussion on #27742.
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:15>
Django
keys
--------------------------------------+------------------------------------
Reporter: Denys Duchier | Owner: nobody
Type: Bug | Status: new
Component: Core (Serialization) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Denys Duchier):
new PR: https://github.com/django/django/pull/8370
I slightly reworked @jpmelos earlier PR #7231 to hopefully avoid the
problem with spurious migrations. When a parent link is being promoted as
a pk, I don't modify that field, but I take note of the promotion in the
inheriting model's _meta (well, Options actually, but that ends up the
same). Then a field is always serialized if it has been promoted, even if
it is marked `serialize=False`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:16>
Django
keys
Reporter: Denys Duchier | Owner: Wai Yi
| Anthony Leung
Type: Bug | Status: assigned
Component: Core | Version: master
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Wai Yi Anthony Leung):
* status: new => assigned
* owner: nobody => Wai Yi Anthony Leung
* has_patch: 0 => 1
Comment:
I'm reviewing the latest PR on https://github.com/django/django/pull/8370
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:17>
Django
keys
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Core | Version: master
(Serialization) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Wai Yi Anthony Leung):
* status: assigned => new
* owner: Wai Yi Anthony Leung => (none)
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:18>
Django
keys
-------------------------------------+-------------------------------------
Reporter: Denys Duchier | Owner: (none)
Type: Bug | Status: new
Component: Core | Version: master
(Serialization) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Anthony King):
posting into here for convenience:
For people that need something working now, I've worked this PR into a
standalone serializer. It works for our use case, however I can't
guarantee it will work in all circumstances.
https://gist.github.com/cybojenix/2476434c513170d2f267cb588a8d645b
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:19>
Django
keys
-------------------------------------+-------------------------------------
| Graham <timograham@…>
Type: Bug | Status: closed
Component: Core | Version: master
(Serialization) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* owner: (none) => Tim Graham <timograham@…>
* resolution: => fixed
Comment:
In [changeset:"cb7860ccedb199cb221c9e084b5104978b246356" cb7860cc]:
{{{
#!CommitTicketReference repository=""
revision="cb7860ccedb199cb221c9e084b5104978b246356"
Fixed #24607 -- Serialized natural keys in multi-table inheritance models.
Thanks João Paulo Melo de Sampaio for the test.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24607#comment:20>