[Django] #23733: Following #23556: manage multi-apps dependencies when squashing
Django
----------------------------+--------------------
Reporter: Twidi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Following ticket #23556 and PR #3280, I found a problem with squashing on
different apps, each one depending on the other at different step
`app1`:
- `1_auto`
- `2_auto`
- `3_auto`, with dependency on `('app2', '2_auto')`
- `4_auto`
With this squash:
- `2_squashed_3` (squash of `2_auto` and `3_auto`)
And `app2`:
- `1_auto`, with dependency on `('app1', '1_auto')`
- `2_auto`
With this squash:
- `1_squashed_2` (squash of `1_auto` and `2_auto`)
With this configuration, there is an error in `build_graph`:
{{{
#!python
File "django/db/migrations/loader.py", line 224, in build_graph
normal[child_key].dependencies.remove(replaced)
KeyError: ('app1', '3_auto')
}}}
The reason is simple, `('app1', '3_auto')` where replaced by `('app1',
'2_squashed_3')`
So I came up with this patch:
{{{
#!diff
diff --git a/django/db/migrations/loader.py
b/django/db/migrations/loader.py
index 9f8be33..0ee3794 100644
--- a/django/db/migrations/loader.py
+++ b/django/db/migrations/loader.py
@@ -219,8 +219,15 @@ class MigrationLoader(object):
for child_key in reverse_dependencies.get(replaced,
set()):
if child_key in migration.replaces:
continue
- normal[child_key].dependencies.remove(replaced)
- normal[child_key].dependencies.append(key)
+ # child_key may appear in a replacement
+ if child_key in reverse_replacements:
+ for replaced_child_key in
reverse_replacements[child_key]:
+ if replaced in
replacing[replaced_child_key].dependencies:
+
replacing[replaced_child_key].dependencies.remove(replaced)
+
replacing[replaced_child_key].dependencies.append(key)
+ else:
+ normal[child_key].dependencies.remove(replaced)
+ normal[child_key].dependencies.append(key)
normal[key] = migration
# Mark the replacement as applied if all its replaced ones
are
if all(applied_statuses):
Note that I loop on reverse_replacements[child_key], but I'm not sure how
we can have many entries here
}}}
I'll provide a PR with two commits:
- one with a test showing the failure
- one with the patch
--
Ticket URL: <https://code.djangoproject.com/ticket/23733>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Twidi | Owner: nobody
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 Twidi):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
My Pull Request: https://github.com/django/django/pull/3446/
--
Ticket URL: <https://code.djangoproject.com/ticket/23733#comment:1>
Django
Reporter: Twidi | Owner: nobody
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
Description changed by Twidi:
Old description:
New description:
With this squash:
And `app2`:
With this squash:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/23733#comment:2>
Django
Reporter: Twidi | Owner: nobody
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
Description changed by Twidi:
Old description:
> Following ticket #23556 and PR #3280, I found a problem with squashing on
New description:
With this squash:
And `app2`:
With this squash:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/23733#comment:3>
Django
Reporter: Twidi | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"fd061b6591ddc0b3045661ff22243919a7c76f17"]:
{{{
#!CommitTicketReference repository=""
revision="fd061b6591ddc0b3045661ff22243919a7c76f17"
Fixed #23733 -- Fixed squashing migrations that depend on multiple apps.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23733#comment:4>