id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
33013	"""AppConfig"" subclass imports found in Django 3.2's automatic ""AppConfig"" discovery"	Joseph Wayodi	nobody	"We are in the process of adding support for Django 3.2 to [https://github.com/django-oscar/django-oscar Oscar].

Oscar [https://github.com/django-oscar/django-oscar/blob/04dd391c900537a61dbb0ae5250ca5c2df6bbc4b/src/oscar/core/application.py#L139 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 [https://github.com/django/django/commit/3f2821af6bc48fa8e7970c1ce27bc54c3172545e#diff-0f8bc657bc27c9f80385c4814c2c2ebc033bda3e03285a7212965309a481cc70R110-R120 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 [https://github.com/django/django/commit/3f2821af6bc48fa8e7970c1ce27bc54c3172545e#diff-0f8bc657bc27c9f80385c4814c2c2ebc033bda3e03285a7212965309a481cc70R110-R120 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. [https://github.com/django/django/commit/3f2821af6bc48fa8e7970c1ce27bc54c3172545e#diff-0f8bc657bc27c9f80385c4814c2c2ebc033bda3e03285a7212965309a481cc70R110-R120 adding the filter] `(not hasattr(mod, ""__all__"") or candidate.__name__ in mod.__all__)`), would work, and is a bit more intuitive.
"	Bug	closed	Core (Other)	3.2	Normal	wontfix		Samir Shah Joseph Wayodi	Unreviewed	0	0	0	0	0	0