#32316: Access __file__ lazily rather than at module level
-------------------------------------+-------------------------------------
Reporter: William Schwartz | Owner: William
Type: | Schwartz
Cleanup/optimization | Status: assigned
Component: Uncategorized | Version: master
Severity: Normal | Resolution:
Keywords: freezers | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by William Schwartz:
Old description:
> 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`.
New description:
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 [https://github.com/django/django/pull/13841 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`
([https://github.com/django/django/pull/13841/commits/54d539c8becbd6d4cfc5bcf01c1be64acd34568d
54d539c])
Remove the undocumented module constant `ROOT` and replace its single use.
=== `django.utils.version`
([https://github.com/django/django/pull/13841/commits/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/django/django/pull/13841/commits/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/django/django/pull/13841/commits/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#comment:1>
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/068.e081ab7d3528f92b6fc2dbf7556b3eb5%40djangoproject.com.
