#32316: Access __file__ lazily rather than at module level
-------------------------------------+-------------------------------------
Reporter: William | Owner: William Schwartz
Schwartz |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: master
Uncategorized |
Severity: Normal | Keywords: freezers
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
So-called ''frozen'' Python environments (such as those mentioned in
#30950) that do not set all modules' `__file__` variable, which
[https://docs.python.org/3/reference/import.html#__file__ 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 from
`django.db.models`.
* Importing **`django.views.debug`** might be avoidable if `DEBUG=False`
or by avoiding all of the views and URLs APIs.
* **`django.utils.version`**'s `get_git_changeset` is called when `django`
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
setting `USE_I18N=False`. Dealing with `trans_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`
([https://github.com/wkschwartz/django/commit/54d539c8becbd6d4cfc5bcf01c1be64acd34568d
54d539c])
Remove the undocumented module constant `ROOT` and replace its single use.
=== `django.utils.version`
([https://github.com/wkschwartz/django/commit/f4edc6e412df87e05ed224525ebf1355410fe268
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`
([https://github.com/wkschwartz/django/commit/07f46b7b26a1a74fa531de834bee240d576b7f85
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`
([https://github.com/wkschwartz/django/commit/24aa80ba413eeaaec0f3c2cab2f028e0dcbaf99b
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
[https://docs.djangoproject.com/en/3.1/topics/auth/passwords/#django.contrib.auth.password_validation.CommonPasswordValidator
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32316>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/053.b0bd23020577ae9bcfc2075e76952fde%40djangoproject.com.
