[Django] #21680: Stop supporting models in non-installed apps
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Core | Keywords: app-loading
(Other) | Has patch: 0
Severity: Normal | Needs tests: 0
Triage Stage: | Easy pickings: 0
Unreviewed |
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
The app registry is explicitly designed to support models outside
installed applications.
This doesn't seem particularly useful and makes the code more complicated
than necessary.
However, the consequences on backwards compatibility are hairy. If users
simply add the missing apps to their INSTALLED_APPS settings, their
project could start loading the wrong templates, static files, etc.
This change shouldn't be made lightly.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Severity: Normal | Triage Stage:
Keywords: app-loading | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by mitar):
* cc: mmitar@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:1>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
The 1.7 release notes already warn against this.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:3>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
This could wreak havoc in Django's own tests, as some create arbitrary
model classes.
A good middle ground could be to require that each model:
- is in an installed application, or
- has an explicit app label.
"Being in an installed application" means that the application has been
registered in the app registry before the model class gets created. This
is related to #21719.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:4>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
I'm going to implement a deprecation path for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:5>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
PR: https://github.com/django/django/pull/2161
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:6>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"3326a412ccde9f72e22a070a0b4d922048ed2286"]:
{{{
#!CommitTicketReference repository=""
revision="3326a412ccde9f72e22a070a0b4d922048ed2286"
Deprecated importing a model before loading its application.
Refs #21719, #21680.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:7>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* keywords: app-loading => app-loading 1.9
Comment:
This ticket cannot move forward until Django 1.9.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:8>
Django
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by loic84):
While discussing #19774 on IRC it appeared that
`django.contrib.sites.models.get_current_site()` which is imported in
various places in Django would end up being an issue to anyone who doesn't
have `django.contrib.sites` installed.
If we want to be ready for Django 1.9, we probably should start a
deprecation period in 1.7 to move `get_current_site()` to the package
`__init__.py`.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:9>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* owner: nobody => aaugustin
* status: new => assigned
* severity: Normal => Release blocker
Comment:
Good point.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:10>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* has_patch: 0 => 1
Comment:
PR for contrib.sites: https://github.com/django/django/pull/2214
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:11>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by carljm):
The PR for contrib.sites looks good.
It occurs to me that the ideal resolution for this ticket would be one
where you could safely import models anytime, but they just wouldn't be
available in the app registry or register themselves with the ORM (e.g.
attach related managers to other models) until/unless their app were
installed. (The general principle being that errors on import, or having
to be careful what you import when, is an unfortunate smell due to
reliance on import side effects.) I presume you considered this and
decided it just wasn't feasible to implement? (I haven't looked at the
relevant code in detail recently enough to know.)
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:12>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
No, I didn't consider that. I don't know how complicated it would be. In
general I would like to see if we can get away with the strict approach
and avoid adding complexity to the app registry.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:13>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"9ffab9cee1a5bd1a2f6c326ae970d92526f9a304"]:
{{{
#!CommitTicketReference repository=""
revision="9ffab9cee1a5bd1a2f6c326ae970d92526f9a304"
Moved RequestSite and get_current_site.
Following the app-loading refactor, these objects must live outside of
django.contrib.sites.models because they must be available without
importing the django.contrib.sites.models module when
django.contrib.sites isn't installed.
Refs #21680. Thanks Carl and Loic for reporting this issue.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:14>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Core (Other) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: app-loading 1.9 | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 1 => 0
* severity: Release blocker => Normal
Comment:
This is still open, but postponed until the deprecation completes.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:15>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: aaugustin =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:16>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
#22182 was a duplicate (with a long explanation).
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:17>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by mlavin):
I've created a related ticket which notes that this deprecation exposes an
issue with the defaults defined in global settings. See #22477.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:18>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"4548a282a1c64bb2ed414da3d08617d869cd3740"]:
{{{
#!CommitTicketReference repository=""
revision="4548a282a1c64bb2ed414da3d08617d869cd3740"
Removed contrib.sites.models.RequestSite/get_current_site() aliases.
Per deprecation timeline; refs #21680.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:19>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by timgraham):
Aymeric will look at this ticket and determine if there is there more to
it than promoting
[https://github.com/django/django/blob/bd98926f0eb19d27821a8a7679b42ff46e53e4da/django/db/models/base.py#L109
the existing warning] to an exception.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:20>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by timgraham):
Also need to verify the documentation for `Options.app_label` in
`docs/ref/models/options.txt` is accurate after this change.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:21>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by aaugustin):
I filed #24312 so as not to lose the idea from comment 12.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:22>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
Four tests in the `test_runner` app are failing.
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:23>
Django
Reporter: aaugustin | Owner:
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: app-loading 1.9 | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by aaugustin):
* needs_better_patch: 1 => 0
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/4100
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:24>
Django
-------------------------------------+-------------------------------------
Type: | Augustin <aymeric.augustin@…>
Cleanup/optimization | Status: closed
Component: Core (Other) | Version: master
Keywords: app-loading 1.9 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: new => closed
* owner: => Aymeric Augustin <aymeric.augustin@…>
* resolution: => fixed
Comment:
In [changeset:"1b8af4cfa023161924a45fb572399d2f3071bf5b"]:
{{{
#!CommitTicketReference repository=""
revision="1b8af4cfa023161924a45fb572399d2f3071bf5b"
Disallowed importing concrete models without an application.
Removed fragile algorithm to find which application a model belongs to.
Fixed #21680, #21719. Refs #21794.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21680#comment:25>