Opened 11 years ago

Closed 11 years ago

#21018 closed Cleanup/optimization (fixed)

Make the implications of order in INSTALLED_APPS more consistent

Reported by: Kevin Christopher Henry Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: app-loading
Cc: k@…, Kevin Christopher Henry, mmitar@…, esauro, django@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There are several subsystems that are affected by the order of the apps in INSTALLED_APPS. These include (at least): templates, static files, translations, and management commands. For the first three, the rule is that the first listed app has precedence: it will be used in preference to any app listed later that uses the same name for something (e.g. a template, file, or translation).

In the case of management commands, however, the semantics are just the opposite: precedence is given to apps that are listed later.

The proposal is to fix this so that management commands follow the same ordering rules as the other subsystems. This is an easy fix, but it has implications for backwards compatibility: specifically, for any project that currently has two apps in INSTALLED_APPS that define the same command, the one that is actually used will change. (Note that this does not apply to the case of apps overriding core management commands; they will continue to do so.)

See here for a discussion of the issue.

See #20914 for a proposal to document more thoroughly the issue of order in INSTALLED_APPS.

Change History (16)

comment:1 by Kevin Christopher Henry, 11 years ago

Cc: Kevin Christopher Henry added
Summary: Make consistent the significance of app ordering in INSTALLED_APPSMake the implications of order in INSTALLED_APPS more consistent
Type: BugNew feature

comment:2 by Russell Keith-Magee, 11 years ago

Resolution: duplicate
Status: newclosed

Although it isn't obvious, this looks to me to be a duplicate of #3591 (or, at the very least, an issue that should be considered as part of that work). This ticket is the placeholder for the "app refactor", which resolves a bunch of issues related to the startup sequence and application loading.

comment:3 by Carl Meyer, 11 years ago

Is it useful to close every ticket related to INSTALLED_APPS handling as a duplicate of #3591? #3591 is already a massive bite to chew, which is a large part of why the work on that ticket has not been merged yet (and AFAICT it's not entirely clear if or when it will be.) If anything, #3591 could really benefit from a reduction and clarification of scope, not a creeping scope that encompasses everything about apps.

Fixing this ticket, if we decide to fix it, is a simple code change; I don't see how it requires or benefits from being lumped in with #3591. It's certainly not in any way a duplicate of the original motivating feature request in #3591. If it happens to be also fixed by a hypothetical eventual fix for #3591, great, but it's a separate issue.

All else being equal, "more smaller tickets handled independently" is superior to "fewer big tickets".

comment:4 by Mitar, 11 years ago

Cc: mmitar@… added

comment:5 by Kevin Christopher Henry, 11 years ago

If this is something that should be changed, I would tend to agree with Carl. It's a simple change (one line of code?), and shouldn't hinder any of the work in #3591, so why not do it right away? Especially since this behavior hasn't been, but should be, and is about to be, documented, it would be nice to be able to say the right thing.

If the question of whether or not to change it is complicated and needs to be considered in light of the work being done in #3591, then I suppose it makes sense to merge it into that issue.

comment:6 by Marc Tamlyn, 11 years ago

Component: Core (Other)Core (Management commands)
Resolution: duplicate
Status: closednew
Triage Stage: UnreviewedAccepted
Type: New featureCleanup/optimization
Version: 1.5master

I agree with Carl as well. Whilst this may be covered by app-refactor, it's also a distinct issue in and of itself, which behaves in a counterintuitive manner.

comment:7 by esauro, 11 years ago

Cc: esauro added

comment:8 by Shai Berger, 11 years ago

The correct fix, IMO, is not one line of code, but a way for the "conflicting" apps to interact.

While this probably doesn't make sense for templates, static files, or translations, many overridden management commands end up being additions, or wrappers, to the commands they override. If you have a painter app and a gunman app both defining a new command draw, then there's no useful interaction, but if you have both django_extensions and django_other_extensions extending runserver, then it makes sense for each of them to "call up" -- and they may just work together well.

Another idea is to allow a general mechanism of command aliasing -- say, a setting

COMMAND_ALIASES = {
   'run1' : 'app1.runserver',
   'run2' : 'app2.runserver',
}

Which would take precedence over the INSTALLED_APPS order; then a user could set runserver='my.runserver' to keep the one they like.

(one can easily get carried away and add default command arguments into the alias, but that would belong in a completely different ticket).

comment:9 by Aymeric Augustin, 11 years ago

Regardless of the merits of a generic inheritance, extensibility or namespacing mechanism, it shouldn't stop us from fixing inconsistencies. And even if we create such a mechanism, it would still be a good idea to have consistent defaults.

Are you really opposed to fixing the inconsistency reported here until we have a "correct fix", whatever that is?

Version 0, edited 11 years ago by Aymeric Augustin (next)

comment:10 by Aymeric Augustin, 11 years ago

#21527 is a subset of this ticket.

comment:11 by Aymeric Augustin, 11 years ago

The patch for #3591 doesn't address this, but it guarantees that apps.get_app_configs() honors the order of settings.INSTALLED_APPS.

comment:12 by Aymeric Augustin, 11 years ago

Keywords: app-loading added

comment:13 by Chris Streeter, 11 years ago

Cc: django@… added

in reply to:  5 comment:14 by Aymeric Augustin, 11 years ago

Since there's no test for the current order, no significant opposition to the change, and no suitable deprecation path, I'm going to proceed with this change.

I find "last app wins" more intuitive that "first app wins" but since it's more backwards compatible to standardize on "first app wins" let's do that.


Replying to marfire:

It's a simple change (one line of code?)

I'll let you look at the 100 + 125 lines of diff for the two commits that fixed this issue and figure out why I ended up two orders of magnitude above your estimate. :-)

comment:15 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

In f17d00278ee4a22421dc8148588d94fe7fa17427:

Wiped get_commands() cache when INSTALLED_APPS changes.

Refs #21018, #21688.

comment:16 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 0ce945a67151acf2c58bc35a47f4c3d45ff30085:

Fixed #21018 -- Reversed precedence order for management commands.

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