#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:3>
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/063.ff9f7d7332c22cff198e9a297d90d194%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
