Opened 6 years ago

Last modified 5 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

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 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 (​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 (0)

Note: See TracTickets for help on using tickets.
Back to Top