Opened 3 years ago

Last modified 3 years ago

#23790 new Bug

Possible bad interaction between migration dependencies and relabeling apps

Reported by: Aymeric Augustin Owned by: nobody
Component: Documentation Version: 1.7
Severity: Normal Keywords:
Cc: info+coding@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Markus Holtermann)

As far as I can tell, in migrations, dependencies is a list of (app_label, migration_name).

I'm wondering what happens if one relabels an application with AppConfig.label. How can migrations handle this case?

(I haven't tried to create such a problem. I'm just making a note before I forget.)

Change History (11)

comment:1 Changed 3 years ago by Markus Holtermann

Cc: info+coding@… added
Description: modified (diff)

It partially works. If there are no migrations for an app yet, all migrations will be created with the changed label in the dependencies and all places it's being referenced. But as soon as you already have existing migrations and then change the label of an app, the makemigrations command blows up with:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/markus/Coding/django/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/home/markus/Coding/django/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/markus/Coding/django/django/core/management/base.py", line 390, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/markus/Coding/django/django/core/management/base.py", line 442, in execute
    output = self.handle(*args, **options)
  File "/home/markus/Coding/django/django/core/management/commands/makemigrations.py", line 58, in handle
    loader = MigrationLoader(None, ignore_no_migrations=True)
  File "/home/markus/Coding/django/django/db/migrations/loader.py", line 48, in __init__
    self.build_graph()
  File "/home/markus/Coding/django/django/db/migrations/loader.py", line 295, in build_graph
    _reraise_missing_dependency(migration, parent, e)
  File "/home/markus/Coding/django/django/db/migrations/loader.py", line 265, in _reraise_missing_dependency
    raise exc
  File "/home/markus/Coding/django/django/db/migrations/loader.py", line 291, in build_graph
    self.graph.add_dependency(migration, key, parent)
  File "/home/markus/Coding/django/django/db/migrations/graph.py", line 47, in add_dependency
    parent
django.db.migrations.graph.NodeNotFoundError: Migration testapp.0002_proxy dependencies reference nonexistent parent node ('otherapp', '0001_initial')

The only way I can think of is using the appconfig.name attribute instead of appconfig.label. But that's hard to find a smooth migration path.

Last edited 3 years ago by Markus Holtermann (previous) (diff)

comment:2 Changed 3 years ago by Markus Holtermann

Description: modified (diff)

comment:3 Changed 3 years ago by Aymeric Augustin

I'm afraid we messed up that part of the API and it's too late to fix :-|

Andrew, do you have any thoughts on this?

comment:4 Changed 3 years ago by Aymeric Augustin

Component: UncategorizedMigrations
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:5 Changed 3 years ago by Carl Meyer

To me this seems like a straightforward case of "not every edge case can be automatic." The fix is manual, but simple, right? You just have to replace that app label in all existing migrations that reference it.

comment:6 Changed 3 years ago by Aymeric Augustin

That's inconvenient when the affected app is a third-party app.

comment:7 Changed 3 years ago by Carl Meyer

A third-party app with models changing its label has always meant changing the name of all its database tables, too (unless it uses Meta.db_table to preserve them). So it's always been the case that changing the label of an app with models is a Fairly Backwards Incompatible Thing To Do. I guess what's new in 1.7 is that a) changing app-label is easier than it used to be (no longer implies changing import path), and b) migrations introduce a new layer of label-dependence that isn't fixed by using a backwards-compatible Meta.db_table.

I'm having trouble seeing a way to make this smoother, other than possibly improving the error message (though the current one isn't terrible) and probably adding a note to the docs in the AppConfig section clarifying that changing app_label midstream is a breaking change for any existing installs of that app. Fundamentally, migrations need some way to reliably and persistently reference an app, and that's always been the purpose of app-label.

comment:8 Changed 3 years ago by Aymeric Augustin

Component: MigrationsDocumentation

A warning in the documentation of AppConfig.label is the way to go then.

comment:9 Changed 3 years ago by Carl Meyer

This does mean that, in the new world of required migrations, using AppConfig to relabel third-party apps is pretty much impossible (due to app-label being hardcoded in migration dependencies). So the idea that AppConfig.label can allow using two third-party apps with conflicting labels is dead (if those apps both have models/migrations).

comment:10 Changed 3 years ago by Carl Meyer

In theory, it would be possible for migrations to stop using app-label for dependencies and start using full dotted paths instead? Maybe?

And then since db table names aren't hardcoded in migrations, maybe migrations could adapt to the app being re-labeled? (If you did the re-labeling before running any of its migrations; re-labeling midstream would still require manual table renames.)

This would be a big change, but if we wanted to revive the idea of using AppConfig.label for third-party apps, I think that would be the necessary path.

Using full paths of course means that if the import path for an app changes, you have to edit all its migrations. But maybe that's better, since import path is definitely under the control of the app author, always, unlike app label (since the advent of AppConfig.label).

If we don't make this change to migrations, it seems possible that there are no really compelling use-cases remaining for AppConfig.label, just bugs-in-waiting, and we should consider deprecating and removing it.

comment:11 Changed 3 years ago by Carl Meyer

Last thought: using AppConfig.label for third-party apps with migrations is still possible, I guess - you just have to use MIGRATION_MODULES also, and regenerate all its migrations. Which gets unfortunate if the app is upgraded and ships a new migration; you'd have to always re-create those yourself.

Note: See TracTickets for help on using tickets.
Back to Top