Opened 3 years ago
Closed 3 years ago
#33013 closed Bug (wontfix)
"AppConfig" subclass imports found in Django 3.2's automatic "AppConfig" discovery
Reported by: | Joseph Wayodi | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Samir Shah, Joseph Wayodi | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
We are in the process of adding support for Django 3.2 to Oscar.
Oscar mixes in its apps' configurations into Django's AppConfig
class, and developers using Oscar then subclass these, to make their own modifications:
# my_app/apps.py from oscar.core.application import OscarConfig class MyAppConfig(OscarConfig): # my app modifications
With Django 3.2, for apps added to INSTALLED_APPS
using the dotted Python path to the app package, Django finds all AppConfig
subclasses in the apps.py
(using Python's inspect.getmembers
), including ones imported to be subclassed. django.apps.config.AppConfig
itself does get excluded, but this is not sufficient, as we and the developers using Oscar do a lot of sub classing of these classes.
To solve this, we would have have to do:
# my_app/apps.py from oscar.core import application class MyAppConfig(application.OscarConfig): # my app modifications
And then ask the developers using Oscar to also do the same.
The problem with this is:
- it is not intuitive - Python developers would not typically expect imports to cause side effects like this, which creates the potential for confusing and hard-to-debug issues
- it is flaky - a new developer may innocently refactor imports not realising why they were done that way, and then encounter the confusing issues
- explicit class imports are normally preferred in Python, unless they cause local name clashes, so requiring them for this reason is a bit odd
- Django itself does a check to exclude
django.apps.config.AppConfig
from being detected, which is indicative of the fundamental issue which remains
It would be nice if Django could mitigate this by having a mechanism for deciding which classes are considered (e.g. using something like the abstract
metadata option used for models). In the short term, restricting the scanning for AppConfig
subclasses to those named in the app.py
's __all__
variable, if it has one (by e.g. adding the filter (not hasattr(mod, "__all__") or candidate.__name__ in mod.__all__)
), would work, and is a bit more intuitive.
Change History (6)
comment:1 by , 3 years ago
Cc: | added |
---|
comment:2 by , 3 years ago
comment:3 by , 3 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
I'm going to assume this works. If not, please provide a minimal reproduce and we can reopen. Thanks!
comment:4 by , 3 years ago
Oscar provides a drop-in replacement for Django's default AppConfig
, for use by Oscar itself, and by any projects that depend on it:
# oscar/core/application.py (Oscar's base "AppConfig" subclass) class OscarConfig(AppConfig): # Oscar's extensions to "AppConfig" # oscar/apps/basket/apps.py (one of Oscar's apps) from oscar.core.application import OscarConfig class BasketConfig(OscarConfig): # Oscar's "basket" app configurations # project/settings.py (a project that depends on Oscar) INSTALLED_APPS = [ ... 'oscar.apps.basket', ... ]
With default_app_config
for oscar.apps.basket
set, the following deprecation warning is displayed:
/project-venv/lib/python3.8/site-packages/django/apps/registry.py:91: RemovedInDjango41Warning: 'oscar.apps.basket' defines default_app_config = 'oscar.apps.basket.apps.BasketConfig'. However, Django's automatic detection did not find this configuration. You should move the default config class to the apps submodule of your application and, if this module defines several config classes, mark the default one with default = True. app_config = AppConfig.create(entry)
If default_app_config
for oscar.apps.basket
is removed, Django finds multiple non-default AppConfig
subclasses (the imported OscarConfig
, and its subclass BasketConfig
), and ends up using its default AppConfig
class.
If OscarConfig.default
is set to False
, this attribute gets inherited by its BasketConfig
subclass, and Django finds no default AppConfig
subclass, and ends up using its default AppConfig
class. This would require us to switch to specifying the app in INSTALLED_APPS
using only oscar.apps.basket.apps.BasketConfig
. Or for projects using Oscar to subclass BasketConfig
just to set BasketConfig.default
to True
.
We could set BasketConfig.default
to True
, but this then causes a problem for any project using Oscar, which should in turn be able to subclass BasketConfig
:
# project/basket/apps.py (overriden app in a project that depends on Oscar) from oscar.apps.basket.apps import BasketConfig class OverriddenBasketConfig(BasketConfig): # overridden "basket" app configurations # project/settings.py (a project that depends on Oscar) INSTALLED_APPS = [ ... 'project.basket', ... ]
The default
attribute gets inherited by its OverriddenBasketConfig
subclass, and Django finds multiple default AppConfig
subclasses (the imported BasketConfig
, and its subclass OverriddenBasketConfig
):
RuntimeError: 'project.basket.apps' declares more than one default AppConfig: 'OverriddenBasketConfig', 'BasketConfig'.
Note that the same underlying issue exists with Django's own AppConfig
, which is why it is necessary to check for that special case. What we are asking for is a generic solution to that problem, which replacements for AppConfig
can rely on.
comment:5 by , 3 years ago
Cc: | added |
---|---|
Resolution: | invalid |
Status: | closed → new |
comment:6 by , 3 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Hi Joseph. Thanks for the additional details.
If OscarConfig.default is set to False, this attribute gets inherited by its BasketConfig subclass, and Django finds no default AppConfig subclass, and ends up using its default AppConfig class. This would require us to switch to specifying the app in INSTALLED_APPS using only oscar.apps.basket.apps.BasketConfig. Or for projects using Oscar to subclass BasketConfig just to set BasketConfig.default to True.
This is the right approach in your case, where you have multiple AppConfig classes in play. Set default = False
on the superclass and have users config the explicit app config they want, or set default = True
on the one subclass they need.
It's worth reviewing the discussion on the original PR where this kind of thing was discussed.
If you wish to propose a change it would need to go via the DevelopersMailingList to seek consensus there.
Does setting the new
AppConfig.default
toFalse
not solve this?https://docs.djangoproject.com/en/3.2/ref/applications/#django.apps.AppConfig.default
See also the Configuring Applications docs.