Opened 4 years ago
Last modified 4 years ago
#32316 closed Cleanup/optimization
Access __file__ lazily rather than at module level — at Version 1
Reported by: | William Schwartz | Owned by: | William Schwartz |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | freezers |
Cc: | Tom Forbes | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
So-called frozen Python environments (such as those mentioned in #30950) that do not set all modules' __file__
variable, which need not be defined, cannot even import Django (without some workarounds) because a small number of Django modules use __file__
at the module level, in a class defined at the module level, or in a function that is called automatically upon import.
Five modules that use __file__
like this are likely to be imported when using Django and thereby cause a frozen Python to crash with a NameError
or similar exception.
- Importing
django.forms.renderers
can be avoided only by avoiding both forms and the ORM altogether as it's imported fromdjango.db.models
. - Importing
django.views.debug
might be avoidable ifDEBUG=False
or by avoiding all of the views and URLs APIs. django.utils.version
'sget_git_changeset
is called whendjango
is imported in pre-alpha development versions.- Importing
django.contrib.auth.password_validation
is only avoidable by not using the Auth app. django.utils.translation.trans_real
uses__file__
to find Django's localization files upon activation; this avoidable only by settingUSE_I18N=False
. Dealing withtrans_real
is sufficiently thorny (and, being an English speaker with English-speaking clients, I can avoid it for now) that I will not address it further here except to say that it might need to be part of the larger discussion at #30950.
What this ticket is not
I am not proposing removing use of __file__
at this time. That would require a longer discussion of intended semantics such as #30950. This ticket is only about removing use of __file__
at the module (or class definition) level in Django application code (not test code). Further I am not proposing banning use of __file__
at the module level at this time, hence minimal new tests and no update to the Django coding style documentation. That too would require a longer conversation.
Proposed fixes
I have pushed PR GH-13841 to address the four of those modules other than trans_real
. I dealt with each module's use of __file__
in separate commits to make them easier to discuss and separate/cherry-pick if needed. Below I link to the individual commits as I discuss each of the four modules. These first two are fairly easy, but the second two may require further consideration.
django.forms.renders
(54d539c)
Remove the undocumented module constant ROOT
and replace its single use.
django.utils.version
(f4edc6e)
Treat the lack of module-global __file__
the same as a failure of git log
by returning None
from get_git_changeset
.
django.views.debug
(07f46b7)
The module-level constant CURRENT_DIR
is used only in the module itself and is undocumented, so I'm assuming it's an obscure private symbol that no one will miss. I've replaced it with a module-level private function _builtin_template_path
that refactors and centralizes finding built-in templates for the entire module.
The one tricky part is that #32105 added the html_template_path
and text_template_path
attributes django.views.debug.ExceptionReporter
. I didn't want to disturb #32105's goal of making the template paths easily override-able, so I avoided calling _builtin_template_path
in the class definition by making detecting the presence of the attributes in __init__
and setting defaults there. Alternatives include making the attributes properties with setters or cached properties without setters.
django.contrib.auth.password_validation
(24aa80b)
The CommonPasswordValidator
-class constant DEFAULT_PASSWORD_LIST_PATH
is used only in one place, the class's instance constructor. While the nature of DEFAULT_PASSWORD_LIST_PATH
is not documented, its existence is inside the docs for the constructor's signature. I've changed DEFAULT_PASSWORD_LIST_PATH
from a class constant into an instance attribute. Another possibility is making DEFAULT_PASSWORD_LIST_PATH
be a django.utils.functional.classproperty
.