mezzanine icon indicating copy to clipboard operation
mezzanine copied to clipboard

Mezzanine apps names can clash with project app names

Open iorlas opened this issue 10 years ago • 16 comments

As of Django 1.7, it is required to have unique app label. For example, we have mezzanine.core which app label is core. My project has core app too, so INSTALLED_APPS looks like:

(
    'core',
    'mezzanine.core'
    ...
)

Which leads to expected issue:

Traceback (most recent call last):
  File "./manage.py", line 17, in <module>
    main()
  File "./manage.py", line 14, in main
    execute_from_command_line(argv)
  File "/home/dtomilin/.virtualenvs/lc/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/home/dtomilin/.virtualenvs/lc/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 312, in execute
    django.setup()
  File "/home/dtomilin/.virtualenvs/lc/local/lib/python2.7/site-packages/django/__init__.py", line 18, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/dtomilin/.virtualenvs/lc/local/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: core

So currently I can't install mezzanine on my project. It should be a good idea to create AppConfig for mezzanine apps, which will set labels with mezzanine. prefix. But it should break backward compat, migrations and some 3rd-party code.

And I can't just do this for my application because of same reasons. I gonna try to monkey-patch mezzanine.core AppConfig class, so I will be able to change app label, but I'm sure it will not that good in long term.

iorlas avatar Oct 26 '15 15:10 iorlas

Guys, I had to rename my app name(full night of pain in production environments...), but this issue is still urgent and blocker for existing projects. You really need to solve it.

iorlas avatar Oct 27 '15 12:10 iorlas

Awesome, @sjdines ! But it does nothing, sadly,

https://github.com/django/django/blob/1.8.5/django/apps/config.py#L18

Name will be overridden anyway. But what you need to override to solve this issue is a label: https://github.com/django/django/blob/1.8.5/django/apps/config.py#L28

But you can't simply do this since you will need to update migrations code, models relations paths, etc.

iorlas avatar Oct 29 '15 20:10 iorlas

Suggest not using a conflicting name.

stephenmcd avatar Dec 18 '15 00:12 stephenmcd

But mezzanine is a one which uses possible conflicting names! I'd say better to do it now than later, to change app names to prefixed. Otherwise it WILL be pain for many years. I have 3rd project already, where I'm stuck with this issue and result was to NOT use mezzanine only because of this issue.

We can't develop any project with keeping such things in mind, because these things could be non existing in the moment of project development. So you can create 'core' app and then you got some 3rd party app, which called 'core' too. It is not predictable.

On other side, it is predictable to get conflicts in existing apps when you develop common library/package/module such as mezzanine. So, to avoid this, you will make some prefixes or call package by it's name. Like celery does it, rq, arrow, vanilla-cbv, django rest framework... any of these could be called by common name like 'queue', 'datetime', 'views', 'api'. And this is wrong

Just my 2 cents for your vision of project and common issues, which cannot be solved easily, which could be a blocker for existing projects.

iorlas avatar Dec 18 '15 13:12 iorlas

I haven't encountered this myself, but I'm inclined to agree that a reusable app claiming generic namespaces is a problem. Can't it be resolved by simply extending #1447 to add a label to each AppConfig?

ryneeverett avatar Dec 18 '15 14:12 ryneeverett

@ryneeverett The real problem is what label change makes to app registry: all migrations, relations to mezzanine apps will be invalid since django uses app label as a slug for mappings. This is why this issue cannot be solved easily.

iorlas avatar Dec 18 '15 16:12 iorlas

@iorlas True, it's not to be taken lightly. But it appears that migrating an app label is fairly straightforward and that this issue is worth fixing at some point.

ryneeverett avatar Dec 18 '15 17:12 ryneeverett

@ryneeverett migration is a bit complicated since you will need to run through your project codebase to fix migrations dependencies and models relations... and imports. Project is that it is all about updating project codebase, this cannot be automated via data migration or such things. I mean, it can be done, but too dangerous.

iorlas avatar Dec 18 '15 17:12 iorlas

@ryneeverett And a point to fix this - ASAP, because it will harm end-users anyway and it is not dependent on time. What depends on time is a number of projects which cannot be user with mezzanine, number of rejects.

iorlas avatar Dec 18 '15 17:12 iorlas

you will need to run through your project codebase to fix migrations dependencies and models relations... and imports. Project is that it is all about updating project codebase

@iorlas Perhaps I misunderstand label. Why would changing it involve changing the codebase? I thought the whole point of label was that you could change it without changing any import paths.

ryneeverett avatar Dec 18 '15 17:12 ryneeverett

@ryneeverett app_label is a slug, which Django will use as application slug in registry. So you could do ForeignKey('core.User') or use it as dependency in migrations:

class Migration(migrations.Migration):
    dependencies = [
        ('core', '0001_initial'),
    ]

So when you change app_label, all these links will be invalid. Since you will have some models in your app, you will have invalid dependencies, so django will not run any migrations without it.

iorlas avatar Dec 18 '15 18:12 iorlas

@iorlas Obviously I don't know this stuff as well as you, but the above SO link seems to suggest this is easy:

And if you're using the new migrations, you'll need to change the app name in the existing migrations files and django_migrations table. It might be better to squash migrations first so there's less to edit. – James ... Would be great if you updated your answer to include what James said about upgrading migration files (such as dependency module names)... couldn't figure that out for quite a bit. – ujvl

@uj-in point 6 in the answer its already mentioned. – Srikar Appal

Point 6 being:

(For Django >= 1.7) Update the django_migrations table to avoid having your previous migrations re-run: UPDATE django_migrations SET app='<NewAppName>' WHERE app='<OldAppName>'

ryneeverett avatar Dec 18 '15 18:12 ryneeverett

Thanks all for the feedback, I'll re-open the issue so we can track a long-term fix, which should include an upgrade path for existing users.

stephenmcd avatar Dec 18 '15 23:12 stephenmcd

BTW @iorlas, to re-iterate Ryne's point - it seems like you're deeply across what needs to occur here, so if you'd like to work on the solution that'd be a great help.

stephenmcd avatar Dec 18 '15 23:12 stephenmcd

@ryneeverett it isn't that easy because you need to rename DB tables and be aware of constraints, DB functions and custom SQL you have in your project codebase. This is why it is harder than you think.

iorlas avatar Dec 19 '15 13:12 iorlas

@stephenmcd Right now I'm tight on time, sadly, but... I think, I could fine some spare time to make an attempt to create few migrations, which should be enough to migrate mezzanine data and schema. Hard part - migrate end-users codebase, like editing of migrations and relations.

And yea, this issue solution means rename of current mezzanine application labels from core to prefix+core. Since Django supports custom App settings classes, we can keep folders names and just add app_labels like mezzanine_core, Which isn't obvious in some cases. What do you think?

iorlas avatar Dec 19 '15 13:12 iorlas