Opened 4 years ago
Last modified 4 years ago
#32316 closed Cleanup/optimization
Access __file__ lazily rather than at module level — at Initial Version
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 |
Pull Requests: | |||
Description ¶
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-XXXX 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
.