Consistent handling for app modules with no (or empty) __path__
|Reported by:||Carl Meyer||Owned by:||Carl Meyer|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
This ticket is a followup from discussion that occurred on https://github.com/django/django/pull/2212 in the course of resolving #17304. The description here is lengthy as I've tried to be comprehensive, but really the issue impacts only very unusual edge-case app modules.
Currently in master, if an app module has no
__path__ attribute, its appconfig gets a
None. This is intended to indicate that the app has no directory path that can be used for finding things like templates, static assets, etc. It is likely to break some naive code that iterates over app-configs using their paths. (For instance, the current code in
AppDirectoriesFinder breaks with a
TypeError if any app-config has a
If an app module would have an empty
__path__, currently an obscure
IndexError would be raised.
If an app module has a
__path__ with more than one element in it (namespace package),
ImproperlyConfigured is raised.
All of the above is bypassed if the app uses an
AppConfig subclass that has an explicit
path as a class attribute - in that case, that path is unconditionally used and no path detection occurs.
__file__ attribute is not currently used at all for determining an app's filesystem path.
Here are some types of app-modules that could be referenced in INSTALLED_APPS, and the contents of their
A) An ordinary package, e.g
myapp is a directory containing an
__init__.py. In this case, the module object's
__path__ will be
['/path/to/myapp/'] and its
__file__ will be
B) A namespace package with only a single location. In this case,
__path__ is length-one, just as in (A), and there is no
C) A namespace package with multiple locations.
__path__ is length > 1, and there is no
D) A single-file module, e.g.
'myapp' where there is just
myapp.py. In this case, the module will have no
__path__ attribute, and
__file__ will be
E) A package inside a zipped egg, e.g.
'myapp' where there is a zipped egg
myapp-0.1-py2.7.egg added to
sys.path in a pth file, and the zipfile internally contains a file
myapp/__init__.py. In this case,
__path__ will contain the bogus non-path
__file__ will similarly be a bogus non-path
'/path/to/myapp-0.1-py2.7.egg/myapp/__init__.pyc'. (A package or module within an unzipped egg is no different from case (A) or (D).)
F) A module loaded by some hypothetical custom loader which leaves
So currently for cases (A), (B), and (E) we use the single entry in
__path__ (even though in case (E) that path doesn't exist), and for case (C) we raise
ImproperlyConfigured and require the user to explicitly supply a path on the
AppConfig. For case (D) we set
None, and for case (F) we raise an obscure
IndexError. I consider our handling of cases (D) and (F) sub-optimal.
I think it would be preferable if
appconfig.path were never set to
None. The algorithm I would propose for
appconfig.path is this: (i) if
AppConfig.path is set explicitly, use it, (ii) If
app_module.__path__ exists and is length 1, use its only element, (iii) if
app_module.__file__ exists, use
os.path.dirname(__file__), and (iv) if no path can be deduced from the
__file__ attributes, then raise
ImproperlyConfigured and tell the user they need to set the path explicitly on their
ImproperlyConfigured for any type of app module that we supported in previous versions of Django would be a backwards-incompatible change. As far as I am aware, only cases (A) and (E) were previously explicitly supported, and neither of those would raise
ImproperlyConfigured with my proposal.
Case (D) seems to work in previous versions of Django, though I don't believe that it was ever explicitly supported. In any case, it would be covered in my proposal by the fallback to
Case (F) is a bit vague, and I don't believe is a backwards-compatibility concern - I think my proposal provides reasonable handling of any combination of
__file__ that any custom loader might set.
If my proposal is rejected for whatever reason and we opt to stick with something closer to the status quo in master, then the following actions should be taken to resolve this ticket: set
__path__ exists but is empty to avoid the
IndexError in case (F), and update
contrib.staticfiles to not blow up if it its an
appconfig.path that is
None (and perhaps audit other uses of
appconfig.path in Django to ensure they can handle it being
One issue that will need to be resolved for my proposal to move forward is that currently the test suite instantiates
AppConfig in a number of places with
app_module=None, which currently passes silently but under my proposal would cause
ImproperlyConfigured to be raised. I plan to look into this issue and propose a patch.
Feedback on the concept welcome, in particular if I've missed any important cases.
Change History (7)
comment:1 follow-up: 2 Changed 3 years ago by
|Triage Stage:||Unreviewed → Accepted|
comment:4 Changed 3 years ago by
|Owner:||changed from nobody to Carl Meyer|
|Status:||new → assigned|