#23733: Following #23556: manage multi-apps dependencies when squashing
----------------------------+--------------------------------------
Reporter: Twidi | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------------------------
Description changed by Twidi:
Old description:
> 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
New description:
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#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/063.fd8881405f871b8832bbde669f4ef62b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
