MigrationLoader.load_disk hides ImportError for invalid MIGRATION_MODULES

[Daniel Hahler]

With an invalid module name in MIGRATION_MODULES, you will get a rather confusing InvalidBasesError:

InvalidBasesError: Cannot resolve bases for [<ModelState: 'djangocms_text_ckeditor.Text'>]
This can happen if you are inheriting models from an app with migrations (e.g. contrib.auth)
 in an app with no migrations; see https://docs.djangoproject.com/en/1.8/topics/migrations/#dependencies for more

This is caused by ignoring the ImportError in ​https://github.com/django/django/blob/d72f8862cb1a39934952e708c3c869be1399846e/django/db/migrations/loader.py#L70-L78, which is meant to handle non-existent migrations.

The following patch might fix it:

diff --git i/django/db/migrations/loader.py w/django/db/migrations/loader.py
index a8f4be4..e872294 100644
--- i/django/db/migrations/loader.py
+++ w/django/db/migrations/loader.py
@@ -72,7 +72,9 @@ def load_disk(self):
             except ImportError as e:
                 # I hate doing this, but I don't want to squash other import errors.
                 # Might be better to try a directory check directly.
-                if "No module named" in str(e) and MIGRATIONS_MODULE_NAME in str(e):
+                if ("No module named" in str(e)
+                        and MIGRATIONS_MODULE_NAME in str(e)
+                        and app_config.label not in settings.MIGRATION_MODULES):
                     self.unmigrated_apps.add(app_config.label)
                     continue
                 raise

But then Django's tests itself fail, because they use this "hack":

    settings.MIGRATION_MODULES = {
        # these 'tests.migrations' modules don't actually exist, but this lets
        # us skip creating migrations for the test models.
        'auth': 'django.contrib.auth.tests.migrations',
        'contenttypes': 'contenttypes_tests.migrations',
    }

([Source](​https://github.com/django/django/blob/d72f8862cb1a39934952e708c3c869be1399846e/tests/runtests.py#L142-147))

I've seen this issue multiple times in the context of Django CMS (and its plugins), because in the progress of migrating to Django's migrations they were using modules like djangocms_text_ckeditor.migrations_django which then were renamed.

[Tim Graham]

A related ​pull request.

[Tim Graham]

As Berker suggested in #django-dev, a system check for invalid entries in MIGRATION_MODULES might be a solution.

[Berker Peksag]

I've implemented the system check idea in ​PR #6853. I'm not sure if the check should be part of the database tag or should be moved to a new migrations tag.

[Simon Charette]

What about letting the ImportError propagate if the invalid migration module is explicitly specified through MIGRATION_MODULES instead?

I see the reasoning behind silencing it the default app.migration package is missing but if a module is explicitly specified I would expect an error to be raised just like any other setting.

The MigrationLoader.migrations_module method's return type could be altered to be of the form module_name, explicit and the exception could be re-raised if explicit is truthy.

[Tim Graham]

If it's feasible that sounds like a simpler solution (feel free to uncheck "patch needs improvement" if you reply with a disagreement).

[Simon Charette]

​Proposed approach.

[Simon Charette <charette.s@…>]

In 0d79292:

Fixed #25109 -- Stopped silencing explicitly specified migration modules import errors.

Thanks Tim for the review.