Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#23483 closed Bug (fixed)

Having two dotted app names with the same last part breaks migrations

Reported by: Szilveszter Farkas Owned by: Szilveszter Farkas
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In our Django project we have django.contrib.auth, and our own vendor.auth app listed in INSTALLED_APPS. After upgrading Django to 1.7 final from 1.7b4, I got the following error message after running migrations (or the manage.py test command that runs migrations in this example):

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/commands/test.py", line 50, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/commands/test.py", line 71, in execute
    super(Command, self).execute(*args, **options)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/commands/test.py", line 88, in handle
    failures = test_runner.run_tests(test_labels)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/test/runner.py", line 147, in run_tests
    old_config = self.setup_databases()
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/test/runner.py", line 109, in setup_databases
    return setup_databases(self.verbosity, self.interactive, **kwargs)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/test/runner.py", line 299, in setup_databases
    serialize=connection.settings_dict.get("TEST_SERIALIZE", True),
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/db/backends/creation.py", line 374, in create_test_db
    test_flush=True,
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/__init__.py", line 115, in call_command
    return klass.execute(*args, **defaults)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 160, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/db/migrations/executor.py", line 63, in migrate
    self.apply_migration(migration, fake=fake)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/db/migrations/executor.py", line 91, in apply_migration
    if self.detect_soft_applied(migration):
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/db/migrations/executor.py", line 135, in detect_soft_applied
    apps = project_state.render()
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/db/migrations/state.py", line 57, in render
    self.apps = Apps([AppConfigStub(label) for label in sorted(self.real_apps + list(app_labels))])
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/apps/registry.py", line 56, in __init__
    self.populate(installed_apps)
  File "/Users/szilveszter/Prezi/repo/website-connected/virtualenv/lib/python2.7/site-packages/django/apps/registry.py", line 89, in populate
    "duplicates: %s" % app_config.label)
django.core.exceptions.ImproperlyConfigured: Application labels aren't unique, duplicates: auth

Change History (14)

comment:1 Changed 5 years ago by Szilveszter Farkas

Owner: changed from nobody to Szilveszter Farkas
Status: newassigned

comment:2 Changed 5 years ago by Szilveszter Farkas

I'm preparing a pull request with a regression test. Stay tuned.

comment:4 Changed 5 years ago by Carl Meyer

Resolution: invalid
Status: assignedclosed

App labels are required to be unique, per the documentation: https://docs.djangoproject.com/en/1.7/ref/applications/#django.apps.AppConfig.label

Mostly, I'm quite surprised that this wasn't already a problem for you under 1.7b4.

You should be able to use a custom AppConfig with a label attribute to define an alternative app label for your own auth app, as documented in the above link.

comment:5 Changed 5 years ago by Szilveszter Farkas

Resolution: invalid
Status: closednew

Can you please take a look at my pull request, and check out the regression test yourself? If you remove the line I have added, you'll see the exception, and it shouldn't happen. The app names and labels are both unique in the AppConfig, but the migration framework uses its own AppConfigStubs that generate this issue.

comment:6 Changed 5 years ago by Carl Meyer

I apologize, you're right. It wasn't clear to me from the ticket description that you did have unique app labels on your apps, and I should have looked at the PR more closely.

On further inspection, I really wonder if, in addition to your fix, AppConfigStub ought to pass None rather than label as the first (app_name) argument to AppConfig.__init__. Passing the label as the name is clearly wrong -- label and name are two different things. The only place AppConfig.name is used is in auto-generating a label if needed (which won't be needed after your fix), and in import_models() method, which AppConfigStub overrides anyway. Having a name of None seems better than having an incorrect name, in that it would fail fast rather than be wrong in case something in future tries to use AppConfigStub.name.

Checking now to see if that change has negative effects.

comment:7 Changed 5 years ago by Szilveszter Farkas

Sorry, I should have included more information about the AppConfigs. Please let me know if you want me to make the change you suggested, and I'll update the pull request.

comment:8 Changed 5 years ago by Carl Meyer

Triage Stage: UnreviewedReady for checkin

The app-registry checks for duplicate names, so setting app name to None is not an option without additional changes. I will merge your fix as-is, as it's sufficient to fix this bug, and open another issue to track the potential problem with label-vs-name.

Thanks very much for the report and pull request! Sorry again for the over-hasty ticket closing.

comment:9 Changed 5 years ago by Carl Meyer <carl@…>

Resolution: fixed
Status: newclosed

In 5e32605ce9baebf07b741c9235ea6351d565ce10:

Fixed #23483 -- Prevented ImproperlyConfigured with dotted app names

Made sure the app labels stay unique for the AppConfigStubs, so
migrations wouldn't fail if two dotted app names has the same last part
(e.g. django.contrib.auth and vendor.auth)

comment:10 Changed 5 years ago by Szilveszter Farkas

Thanks for the super fast turnaround! Do you think this could get into the 1.7.x branch as well? I think it qualifies as a bug fix for a major new feature in the series.

comment:11 Changed 5 years ago by Carl Meyer

Yes, I agree; I am planning to backport it. Just making notes on the other issue first before I forget.

comment:12 Changed 5 years ago by Carl Meyer <carl@…>

In ac1adfbe4a822a3347401f574e64f83e36128cec:

[1.7.x] Fixed #23483 -- Prevented ImproperlyConfigured with dotted app names

Made sure the app labels stay unique for the AppConfigStubs, so
migrations wouldn't fail if two dotted app names has the same last part
(e.g. django.contrib.auth and vendor.auth)

Backport of 5e32605ce9 from master.

comment:13 Changed 5 years ago by Tim Graham <timograham@…>

In 08042f1a73ce6b0feca7a80fc6afc121928b8702:

Added 1.7.1 release notes for refs #23483.

comment:14 Changed 5 years ago by Tim Graham <timograham@…>

In 4b8798cfb9465af1b65ec3cba7ee10b8f969a363:

[1.7.x] Added 1.7.1 release notes for refs #23483.

Backport of 08042f1a73 from master

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