Validate that values in INSTALLED_APPS are unique

[Piotr Czachur <zimnyx@…>]

Docs for INSTALLED_APPS says:
"A tuple of strings designating all applications that are enabled in this Django installation. Each string should be a full Python path to a Python package that contains a Django application, as created by django-admin.py startapp."
Fact that application name must be unique in INSTALLED_APPS otherwise Django will go crazy isn't mentioned at all.
It really should be in bold.

[Russell Keith-Magee]

There may also be some room validate this formally on startup, but at the very least, this should be documented.

[sbj3]

Replying to Piotr Czachur <zimnyx@gmail.com>:

Docs for INSTALLED_APPS says:
"A tuple of strings designating all applications that are enabled in this Django installation. Each string should be a full Python path to a Python package that contains a Django application, as created by django-admin.py startapp."
Fact that application name must be unique in INSTALLED_APPS otherwise Django will go crazy isn't mentioned at all.
It really should be in bold.

[sbj3]

Added clarifying sentence.

[sbj3]

Attached patch. (Closed ticket by mistake. noobi)

[Russell Keith-Magee]

(In [13219]) Refs #12288 -- Clarified that application names must be unique. Thanks to Piotr Czachur for the report.

[Russell Keith-Magee]

(In [13224]) [1.1.X] Refs #12288 -- Clarified that application names must be unique. Thanks to Piotr Czachur for the report.

Backport of r13219 from trunk.

[Russell Keith-Magee]

The uniqueness has now been reinforced in docs; I'm leaving this ticket open as a placeholder for the need for better validation of the contents of INSTALLED_APPS

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Tim Graham]

Updating summary per Russ's last comment.

[Susan Tan]

@Timo, I'm fairly new to writing tests in Django. Should the new test be added in tests/settings_tests?

In principle, the test would look like this (and feel free to correct me if I'm wrong):

from settings import INSTALLED_APPS

def test_unique_installed_apps(TestCase):
   self.assertEqual(len(INSTALLED_APPS), len(set(INSTALLED_APPS))

I'm not quite sure of the package names and the details that this test would require. What are your thoughts?

[Tim Graham]

This would be a new feature (validating that the values in a user's settings.INSTALLED_APPS are unique), not just a test. I'm not sure where the best place to put this new code would be.

[Susan Tan]

Perhaps I should ask on the core django mentorship list, of which Jeremy is in charge of?

[Simon Charette]

As answered on ​Django Core Membership (message has been deleted for no reasons):

There's already a check in django.conf.Settings to make sure INSTALLED_APPS is not a string so I'd suggest you add this check at the same place.

Concerning the tests I suggest you take a look at the settings_tests test application. However you might have to move the check to BaseSettings.__setattr__ if you want to use the testing strategy used in settings_tests.TrailingSlashURLTests.

[Susan Tan]

Thanks for the advice. I understand now. I have an additional question related to debugging. I often like to put print statements to check the values and the variable types. Is there a way to do that in the django project? For example, in django/conf/init.py, I want to print out the result of line 124: mod = importlib.import_module(self.SETTINGS_MODULE) Are there debugging tools I should be aware of that can be particularly helpful for this project?

[Susan Tan]

I've made a PR. This PR is in initial stages of development: ​https://github.com/django/django/pull/1445 I haven't tested any of this code that I wrote. It's a work in progress. Now that I think more about this ticket, am I really writing a test for a test?

[Tim Graham]

Regarding debugging, if you put a print statement in there, that code will be run when using runserver for example.

The logic in the PR looks correct. As Simon said, if you put the logic in BaseSettings.__setattr__, you should be able to use a similar testing style as settings_tests.TrailingSlashURLTests.

[Susan Tan]

Updated the PR based on the feedback here. Unit tests pass. Let me know if there's anything I left out or forgot to consider. PR is here: ​https://github.com/django/django/pull/1445/files

[Tim Graham <timograham@…>]

In 2ac89012d8ff750ea5443b6f6f347dacb697e059:

Fixed #12288 -- Added unique validation for INSTALLED_APPS

[Dominic Rodger]

There's a typo in the test class name - tiny pull request at ​https://github.com/django/django/pull/1461.

[Tim Graham <timograham@…>]

In 9c711ee3a6a638add26d19dad70447c981371598:

Fixed test failures on Python 3 - refs #12288

[Tim Graham]

Looking at the ​documentation, it appears we didn't make the validation strict enough: "The application names (that is, the final dotted part of the path to the module containing models.py) defined in INSTALLED_APPS must be unique. For example, you can’t include both django.contrib.auth and myproject.auth in INSTALLED_APPS."

[Ivan Kolodyazhny]

I've updated tests and fix for this ticket in ​https://github.com/django/django/pull/1626

[Tim Graham <timograham@…>]

In c1ec08998d1b690855d5e69a1f4d9d2f01d44ae6:

Fixed #12288 -- Validated that app names in INSTALLED_APPS are unique

[loic84]

Latest commit is very much backward incompatible and is IMO a bad idea.

I can't run master anymore, and fixing it would require renaming tons of packages.

"X.Y.Z" is *not* the same as "A.B.Z".

Also lot of third party apps would now be conflicting with each other, particularly the namespaced ones like "mezzanine" or "armstrong".

What's the gain exactly? I've been using this for years and never encountered any issues.

[Anssi Kääriäinen]

Django assumes in some places that app_name.model_name is unique key (IIRC things like contenttypes, permissions and app-cache assume this). I think if you happen to have somepackage.app1.User and otherpackage.app1.User you will have problems.

That being said backwards compatibility could be a bigger concern that making sure the above doesn't happen.

[Marc Tamlyn]

Also worth noting that this patch does not actually prevent the issue discussed - you can just as easily create a User model with an app_label of auth, and have just as much of a problem. Can we not apply this validation at the app cache level if this is what we're trying to avoid? The first patch is good as it catches easy mistakes, the second seems overkill. I'm not sure it's a pattern we should encourage, perhaps print a warning on server start, but I think it should be allowed.

[Tim Graham <timograham@…>]

In 886bb9d8780303b4c8f45c55e0ac0a6b644b73af:

Revert "Fixed #12288 -- Validated that app names in INSTALLED_APPS are unique"

This reverts commit c1ec08998d1b690855d5e69a1f4d9d2f01d44ae6.

There are backwards compatability concerns with this.

[Tim Graham]

Here's a discussion on the mailing list that looks into why app names have to be unique.

​https://groups.google.com/forum/#!topic/django-developers/gzBWU_fUdgc/discussion

For now, I've reverted the latest commit given the backwards incompatibility concerns.

[Tim Graham]

Closing this -- if someone feels the app cache validation described by Marc in comment 34 is worthwhile, please open a new ticket.

[Aymeric Augustin]

I fixed this properly in the app loading project, after introducing the ability to relabel an application, which provides an exit path for people who rely on the current lax behavior.