Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29506 closed Cleanup/optimization (fixed)

Misleading migrate "App 'apps.somethings' does not have migrations." error message with nested apps

Reported by: oliver Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by oliver)

If you have a couple of nested apps, or apps that are not available directly in the top-level namespace and you use migrate command, and pass the full dotted path to this command, the error message is somewhat misleading:

CommandError: App 'apps.somethings' does not have migrations.

In this case, the somethings app have migrations, but migrate command expecting just app_label.

And there is the other misleading error message.
If you use migrate command and pass nonexistent app in your INSTALLED_APP, the error message is somewhat misleading:

CommandError: App 'nonexistent_app' does not have migrations.

In this case, the nonexistent_app does not exist, but the error message make user think that nonexistent app just don't have migrations.

Currently, makemigrations command check for existence and validity of app. As follows

       # Make sure the app they asked for exists
        app_labels = set(app_labels)
        bad_app_labels = set()
        for app_label in app_labels:
            try:
                apps.get_app_config(app_label)
            except LookupError:
                bad_app_labels.add(app_label)
        if bad_app_labels:
            for app_label in bad_app_labels:
                if '.' in app_label:
                    self.stderr.write(
                        "'%s' is not a valid app label. Did you mean '%s'?" % (
                            app_label,
                            app_label.split('.')[-1],
                        )
                    )
                else:
                    self.stderr.write("App '%s' could not be found. Is it in INSTALLED_APPS?" % app_label)
            sys.exit(2)

How about checking that in migrate command?

This ticket is very similar to https://code.djangoproject.com/ticket/29469.

But it is about migrate not makemigrations.
Please read this ticket.

Change History (7)

comment:1 by oliver, 6 years ago

https://github.com/django/django/pull/10061

I added code as follows

+        # Make sure the app they asked for exists
+        if options['app_label']:
+            app_label = options['app_label']
+            try:
+                apps.get_app_config(app_label)
+            except LookupError:
+                if '.' in app_label:
+                    raise CommandError(
+                        "'%s' is not a valid app label. Did you mean '%s'?" % (
+                            app_label,
+                            app_label.split('.')[-1],
+                        )
+                    )
+                else:
+                    raise CommandError("App '%s' could not be found. Is it in INSTALLED_APPS?" % app_label)
Last edited 6 years ago by oliver (previous) (diff)

comment:2 by oliver, 6 years ago

Description: modified (diff)

comment:3 by Claude Paroz, 6 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Thanks for the report and the proposed patch. I'll suggest a slightly different approach in an upcoming patch, as I think the fix for #29469 was not optimal either.

comment:4 by Claude Paroz, 6 years ago

Patch needs improvement: unset

My suggested PR.

comment:5 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In c723a1ff:

Fixed #29506 -- Added validation for migrate's app_label option.

Thanks MyungSeKyo for the report and the initial patch.

comment:6 by Tim Graham <timograham@…>, 6 years ago

In fc266151:

Refs #29506 -- Added validation for squashmigrations' app_label option.

in reply to:  5 comment:7 by oliver, 6 years ago

Thank you too.
I learned a lot from your code!

Replying to Tim Graham <timograham@…>:

In c723a1ff:

Fixed #29506 -- Added validation for migrate's app_label option.

Thanks MyungSeKyo for the report and the initial patch.

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