Opened 2 years ago

Closed 2 years ago

#21018 closed Cleanup/optimization (fixed)

Make the implications of order in INSTALLED_APPS more consistent

Reported by: marfire Owned by: nobody
Component: Core (Management commands) Version: master
Severity: Normal Keywords: app-loading
Cc: k@…, marfire, 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


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 Changed 2 years ago by marfire

  • Cc marfire added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Make consistent the significance of app ordering in INSTALLED_APPS to Make the implications of order in INSTALLED_APPS more consistent
  • Type changed from Bug to New feature

comment:2 Changed 2 years ago by russellm

  • Resolution set to duplicate
  • Status changed from new to closed

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 Changed 2 years ago by carljm

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 Changed 2 years ago by mitar

  • Cc mmitar@… added

comment:5 follow-up: Changed 2 years ago by marfire

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 Changed 2 years ago by mjtamlyn

  • Component changed from Core (Other) to Core (Management commands)
  • Resolution duplicate deleted
  • Status changed from closed to new
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from New feature to Cleanup/optimization
  • Version changed from 1.5 to master

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 Changed 2 years ago by esauro

  • Cc esauro added

comment:8 Changed 2 years ago by shai

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

   '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 Changed 2 years ago by aaugustin

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 will 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?

Last edited 2 years ago by aaugustin (previous) (diff)

comment:10 Changed 2 years ago by aaugustin

#21527 is a subset of this ticket.

comment:11 Changed 2 years ago by aaugustin

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 Changed 2 years ago by aaugustin

  • Keywords app-loading added

comment:13 Changed 2 years ago by streeter

  • Cc django@… added

comment:14 in reply to: ↑ 5 Changed 2 years ago by aaugustin

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 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

In f17d00278ee4a22421dc8148588d94fe7fa17427:

Wiped get_commands() cache when INSTALLED_APPS changes.

Refs #21018, #21688.

comment:16 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 0ce945a67151acf2c58bc35a47f4c3d45ff30085:

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

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