[Django] #22477: Incompatible Global Settings
Django
-------------------------------+--------------------
Reporter: mlavin | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The app-loading refactor has deprecated importing a model which is not in
the installed apps #21680. This leads to some incompatible defaults in the
global settings defaults. In the defaults {{{INSTALLED_APPS}}} is empty
but {{{MIDDLEWARE_CLASSES}}} contains
{{{'django.contrib.sessions.middleware.SessionMiddleware'}}} and
{{{'django.contrib.auth.middleware.AuthenticationMiddleware'}}} which rely
on the related apps being in the {{{INSTALLED_APPS}}}. Since most users
create their initial settings via {{{startproject}}} this isn't an issue
but it still makes for a poor default. It also implies a coupling or
dependency between Django core and {{{contrib.auth}}} and
{{{contrib.sessions}}} which doesn't actually exist.
My recommendation would be to remove any contrib middleware from the
{{{MIDDLEWARE_CLASSES}}} in the global settings but continue to set them
in the {{{startproject}}} template which also sets an appropriate
{{{INSTALLED_APPS}}} setting. Since this setting is created by default in
{{{startproject}}} I don't think this raises any major backwards
incompatibility issues.
--
Ticket URL: <https://code.djangoproject.com/ticket/22477>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+--------------------------------------
Reporter: mlavin | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Indeed, this is inconsistent, but it should hardly ever crop up in
practice.
Fixing it technically backwards-incompatible for people who:
- have removed the declaration from their settings because they're using
the global default
- are importing the global default and tweaking it, for instance by
appending some middleware to the list.
I believe the global settings could use a much more fundamental overhaul
and stop pretending they're the documentation for settings -- we have
ref/settings.txt for this purpose.
I don't have a strong opinion on how to deal with this. I usually prefer
tackling problems globally rather than piecewise. But changing a large
number of defaults could be unpopular!
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:1>
Django
-------------------------------+--------------------------------------
Type: Uncategorized | Status: assigned
Component: Uncategorized | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mlavin):
* owner: nobody => mlavin
* status: new => assigned
Comment:
The first case of people who have removed the declaration could be handled
through the system checks similar to the warning for the change to the
{{{TEST_RUNNER}}} setting:
https://github.com/django/django/blob/master/django/core/checks/compatibility/django_1_6_0.py
I would personally rather fix them piecewise at least on the ticket basis.
I know there are larger movements within the community to try to remove or
simplify the settings (there are currently 6 settings just for the CSRF
cookie) but sweeping changes are difficult to move through the process.
Waiting on a more general solution to global settings is letting better be
the enemy of done. To me this is a clear inconsistency which is simple to
fix and document, it's unlikely to impact many users and some (though not
all) of the rare users who might be bitten by this change can be warned
through the system checks.
I'll put together a PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:2>
Django
-------------------------------+--------------------------------------
Reporter: mlavin | Owner: mlavin
Type: Uncategorized | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mlavin):
* has_patch: 0 => 1
Comment:
Added a PR against 1.7.x https://github.com/django/django/pull/2591. If
you feel like this shouldn't be back-ported I'll fix to submit vs master.
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: mlavin | Owner: mlavin
Type: Cleanup/optimization | Status: assigned
Component: Uncategorized | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
It's a bit late in the 1.7 release cycle to make this change, so I suggest
we do it in 1.8. Note that PRs should always be against master anyway; we
then backport from there.
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: mlavin | Owner: mlavin
Type: | Status: assigned
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-2
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 1 => 0
* version: master => 1.7-beta-2
* component: Uncategorized => Core (Other)
* severity: Normal => Release blocker
Comment:
The fact that this results in deprecation warnings when testing reuseable
apps with minimal settings came up in IRC so I think we should consider
including it in 1.7 after all. [https://github.com/django/django/pull/2790
Updated PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: mlavin | Owner: mlavin
Type: | Status: assigned
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-2
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mlavin):
Yes if you have a runtests.py for a reusable app which doesn't include
{{{contrib.auth}}} or {{{contrib.sessions}}} in the {{{INSTALLED_APPS}}}
and doesn't change the {{{MIDDLEWARE_CLASSES}}} but does hit a view by
running through the middleware stack (i.e with the test client) you'll see
these deprecation warnings. With this change you'll instead see the system
check which for 1.7 at least is louder then the
{{{RemovedInDjango19Warning}}}. The fix for it in either way is to be
explicit about the required set of {{{MIDDLEWARE_CLASSES}}} for running
the tests and if you need the auth or session middleware then those should
be in the {{{INSTALLED_APPS}}} as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: mlavin | Owner: mlavin
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d94de802d34c858805a01d3c699799aebc247ec3"]:
{{{
#!CommitTicketReference repository=""
revision="d94de802d34c858805a01d3c699799aebc247ec3"
[1.7.x] Fixed #22477 -- Removed contrib middleware from the global
settings defaults.
Also added a compatibility check for changed middleware defaults.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: mlavin | Owner: mlavin
Type: | Status: closed
Cleanup/optimization | Version:
Component: Core (Other) | 1.7-beta-2
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"4696cd9671243958fd4a596631d75b3cbca325c3"]:
{{{
#!CommitTicketReference repository=""
revision="4696cd9671243958fd4a596631d75b3cbca325c3"
Fixed #22477 -- Removed contrib middleware from the global settings
defaults.
Also added a compatibility check for changed middleware defaults.
Forwardport of d94de802d3 from stable/1.7.x
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22477#comment:8>