[Django] #33272: Non-installed model error isn't raised if the module is in the submodule of an installed app
Django
an installed app
-------------------------------------+-------------------------------------
Reporter: David | Owner: nobody
Seddon |
Type: | Status: new
Uncategorized |
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Imagine a Django project with two apps:
- `foo` - included in `INSTALLED_APPS`.
- `foo.bar` - not included in `INSTALLED_APPS`.
If a model from `foo.bar` is imported, I would expect Django to error
along these lines:
{{{
RuntimeError: Model class foo.bar.SomeModel doesn't declare an explicit
app_label and isn't in an application in INSTALLED_APPS.
}}}
However, instead it imports successfully. Only when the application
queries the database will it run into an error, as the table for the model
will never have been created.
The root cause is `get_containing_app_config`, which just looks for the
innermost installed application that is a parent of the model.
I wonder if there is a way to detect that the model isn't actually part of
an installed app?
--
Ticket URL: <https://code.djangoproject.com/ticket/33272>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
an installed app
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: James Owen (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33272#comment:1>
Django
an installed app
-------------------------------------+-------------------------------------
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
Thanks for this ticket, however it works as
[https://docs.djangoproject.com/en/stable/topics/db/models/#using-models
documented]:
> ''"Once you have defined your models, you need to tell Django you’re
going to use those models. Do this by editing your settings file and
changing the INSTALLED_APPS setting to add the name of the module that
contains your models.py."''
Moreover, it should be possible to define models in a different module
however you have to manually propagate changes to your database schema. I
don't see any reason to raise an error in such case. If you're having
trouble understanding how Django works, see
TicketClosingReasons/UseSupportChannels for ways to get help.
--
Ticket URL: <https://code.djangoproject.com/ticket/33272#comment:2>
Django
an installed app
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Seddon):
Thanks for your reply and sorry if I haven't explained the issue well
enough. I'm going to give it another try.
(Context: I've been a full time Django dev for over a decade so I'm not
coming to this as a newbie.)
My argument is that Django has an inconsistent picture of what models
belong to an app.
In the above example, let's imagine we run `manage.py makemigrations foo`.
Django will not pick up anything from `foo/bar/models.py`, because they
are not imported from `foo/models.py`. In that version of things, Django
(rightly I think) does not view those models as part of the app.
However, if, elsewhere, we import a model from `foo/bar/models.py`, the
[https://github.com/django/django/blob/ca9872905559026af82000e46cde6f7dedc897b6/django/apps/registry.py#L250
get_containing_app_config] method will treat the model as in `foo`
application, even though it wasn't (and shouldn't have been) registered.
In my opinion the `Model class x doesn't declare an explicit app_label...`
exception should be raised whenever a model is imported that wasn't
registered in the course of Django bootstrapping. It shouldn't matter that
that module happens to be in a subpackage of an installed app - that seems
to me to be too much of a blunt instrument.
This behaviour causes us problems in our Django application, because we
have apps that are only sometimes installed, that are submodules of other
installed apps.
--
Ticket URL: <https://code.djangoproject.com/ticket/33272#comment:3>
Django
an installed app
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hey David.
Perhaps you could propose a change that addresses your issue? When
thinking about this yesterday, one consideration is that folks may be
[https://docs.djangoproject.com/en/3.2/topics/db/models/#organizing-
models-in-a-package organising their models as a package], so that would
need to be correctly handled. These things are often easier to consider
with a test case and change in hand, especially if small.
--
Ticket URL: <https://code.djangoproject.com/ticket/33272#comment:4>
Django
an installed app
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Seddon):
Hi Carlton,
I've looked at this in a bit more detail.
From what I understand, `get_containing_app_config` is called to determine
the containing app the first time a model is imported. If this happens
during the bootstrap process (via models autodiscovery), then all well and
good: the model gets registered as part of its containing app.
The problem comes if an ''unregistered'' model class is inadvertently
imported later as part of some runtime process. In this case it will go
through the same registration process (but later than it should do),
giving an inconsistent picture of which models are registered .We don't
really want a model's registration to be triggered, say, by some incoming
web request - or even just because running the webserver involves a larger
import graph than running `manage.py makemigrations`.
Ideally, the model registration process should be able to tell whether
it's being called late. Might one option be for the
[https://github.com/django/django/blob/main/django/db/models/base.py#L74
model registration code] to insist on `apps.models_ready` being `False`
before registering a model? (If it's `True`, it could raise a
`RuntimeError`)?
I don't believe this would get in the way of organizing models as packages
(that's something we do a lot) as the models would still be registered
during the bootstrap process.
--
Ticket URL: <https://code.djangoproject.com/ticket/33272#comment:5>