Opened 6 years ago

Closed 6 years ago

#23484 closed Cleanup/optimization (fixed)

AppConfigStub.name is set to the app-label, which is not the same thing

Reported by: Carl Meyer Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In investigating #23483, I noticed that AppConfigStub (whose __init__ method receives only an app label) passes on this app-label to AppConfig.__init__ as the app_name argument.

This is clearly wrong, as the app label and app name are two different things (if not customized, the latter should be the full dotted path, and the former the final segment only). In practice it doesn't cause any problems, because the app name is used in very few places. It is used to construct the default app-label; in the context of AppConfigStub, this was the cause of #23483, and after the fix for #23483 it isn't used for this anymore in the AppConfigStub case. And it is used in AppConfig.import_models, which is overridden by AppConfigStub anyway.

It would seem more robust to pass in None when we don't know the actual app-name rather than using the app-label as a substitute (in fact, that's what AppConfigStub used to do prior to c40209). However, this now runs afoul of the app-registry's check for duplicate app names.

Options:

  1. Do nothing, because it's not currently broken. Maybe add a comment clarifying that we know label technically is the wrong thing to pass in here, but it's good enough in practice and we don't have anything better.
  1. Pass None, and filter out app-names of None in the app registry before checking for dupes.
  1. Pass something explicit like "fake app name for app label %s" % label.

I think option (2) is overly invasive, given that the fix is just for future robustness, not something that's currently broken. I'd probably favor (3), but would be fine with (1) too.

Thoughts?

Change History (1)

comment:1 Changed 6 years ago by Carl Meyer <carl@…>

Resolution: fixed
Status: newclosed

In afa119918c5584e7ca169c80720fd11d8b3436d0:

Fixed #23484 -- Add comment in AppConfigStub clarifying app name vs app label.

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