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 William Schwartz)

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 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-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.

Change History (1)

comment:1 by William Schwartz, 4 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top