[Django] #24093: Migration writer includes kwargs for operations even if they don't occur in the deconstruct output
Django
the deconstruct output
---------------------------------+---------------------
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------
In 21e21c7bc2b8bf7ae127e2aa75048a60d05a6e0f (#23844) we introduced a way
to deterministicly deconstruct migration operations that don't rely on
the argument ordering. However, the migration writer still includes all
arguments of an operations, since it inspects the operation's `__init__()`
function (see
[https://github.com/django/django/blob/12bf42ae0db752bf4a4387d6be7276cd145f59d1/django/db/migrations/writer.py#L51-L52
django/db/migrations/writer.py#L51-L52]).
This behavior makes migrations generated under 1.8 practically unusable on
1.7, even if they don't use managers in migrations. Due to this I classify
this issue as a release blocker.
See also #23892 for a discussion about the forwards / backwards
compatibility of migrations.
--
Ticket URL: <https://code.djangoproject.com/ticket/24093>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
the deconstruct output
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0
Comment:
PR: https://github.com/django/django/pull/3853
--
Ticket URL: <https://code.djangoproject.com/ticket/24093#comment:1>
Django
the deconstruct output
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* severity: Release blocker => Normal
* stage: Unreviewed => Accepted
Comment:
As I said on #23892, I don't think we should attempt to make migrations
forwards-compatible, and I actually see some advantage to having them fail
fast if you try rather than "maybe work depending which features you use."
So I don't believe this is high priority or should be a release blocker.
However, I do think this is worth fixing, not for forward-compatibility
but just for the sake of cleaner migration files. And if we want it to
"fail fast" when you use newer migrations on older Django, we should do
that in a more explicit way anyway. So I'm +0 on merging this change.
--
Ticket URL: <https://code.djangoproject.com/ticket/24093#comment:2>
Django
the deconstruct output
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
It's not about forwards compatibility, at least I didn't consider and
think about it. The output is just not the way as it was intended by the
change.
--
Ticket URL: <https://code.djangoproject.com/ticket/24093#comment:3>
Django
the deconstruct output
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carl Meyer <carl@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"862ea825b5073588efafd5b7eed349ad098b5fe1"]:
{{{
#!CommitTicketReference repository=""
revision="862ea825b5073588efafd5b7eed349ad098b5fe1"
Fixed #24093 -- Prevented MigrationWriter to write operation kwargs that
are not explicitly deconstructed
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24093#comment:4>