Code

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#6677 closed (wontfix)

Duplicates apps in settings.INSTALLED_APPS

Reported by: trbs Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords: settings, INSTALLED_APPS
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Duplicated in settings.INSTALLED_APPS are not removed automaticly.

As my project grows and the application lists grows with it, i tent to split of parts into a different python project
which i include in every django site i build. For the settings.py of the website it includes all the defaults from myproject.conf.settings
but this sometimes leads to duplicated in the INSTALLED_APPS list.

The duplicates are a human error and should not happen (or be avoided) but that doesn't mean it won't happen someday (specially with different developers working at the same site).

Attached to this ticket is a patch to check for duplicates while creating the django settings object in django.conf.init
I think that while developers should not put duplicates in there list, checking it while looping through DJANGO_SETTINGS_MODULE.INSTALLED_APPS and handling it explicitly is a good idea. (zen: Explicit is better then implicit)

The implementation currently uses a Set object, but could also be written with an
condition:

...
          if not app in new_installed_apps:
              new_installed_apps.append(app)
...

Would love to hear some comments on this :-)

As i think currently this is not checked/handled at all, the list just contains duplicate entries and that
could break some things, like registration in newformsadmin or databrowse.

Personally i believe it should check and either automaticly remove duplicates or raise some exception indicating
that duplicate values are not allowed.

Attachments (1)

django_conf_init.patch (1.3 KB) - added by trbs 6 years ago.

Download all attachments as: .zip

Change History (4)

Changed 6 years ago by trbs

comment:1 Changed 6 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

Nah, cleaning up for sloppy programmers isn't something Django should do. Don't put duplicates in INSTALLED_APPS.

comment:2 follow-up: Changed 6 years ago by trbs

Jacob, then why not raise an exception when a duplicate is found ?

Like said in the ticket, i also feel that Django should not cleanup after sloppy programmers, but it would be nice if it would be checked.
The thought was that bailing out with a DuplicateInstalledAppsError would be much clearer then continuing and getting a deeper error raised which may not always be obviously pointing to the duplicate apps in the list.

Anyways thanks as always for the great work you guys do :)

comment:3 in reply to: ↑ 2 Changed 6 years ago by jacob

Replying to trbs:

Jacob, then why not raise an exception when a duplicate is found ?

That's not a bad idea per se, but if we added checks for every possible mistake a programmer could make we'd be here all day. I'd rather just rely on folks being generally smart enough not to do things that don't work.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.