[Django] #25624: Autoreload fails if jinja2.ModuleLoader used
Django
----------------------------+----------------------------------------------
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Severity: Normal | Keywords: autoreload, jinja2, ModuleLoader
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+----------------------------------------------
It happens because `jinja2.ModuleLoader` injects a weakref to a fake
module into a sys.modules:
https://github.com/mitsuhiko/jinja2/blob/f6b654de615de61a9ca79e7ccecf7b4a8bf90ec0/jinja2/loaders.py#L439-L449
So, because weakref is not a hashable object, autoreload fails.
{{{
Traceback (most recent call last):
File "bug/manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/__init__.py", line 351, in
execute_from_command_line
utility.execute()
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/__init__.py", line 343, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/base.py", line 394, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/commands/runserver.py", line 49, in
execute
super(Command, self).execute(*args, **options)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/base.py", line 445, in execute
output = self.handle(*args, **options)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/commands/runserver.py", line 88, in handle
self.run(**options)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-
packages/django/core/management/commands/runserver.py", line 97, in run
autoreload.main(self.inner_run, None, options)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-packages/django/utils/autoreload.py",
line 336, in main
reloader(wrapped_main_func, args, kwargs)
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-packages/django/utils/autoreload.py",
line 302, in python_reloader
reloader_thread()
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-packages/django/utils/autoreload.py",
line 278, in reloader_thread
change = fn()
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-packages/django/utils/autoreload.py",
line 207, in code_changed
for filename in gen_filenames():
File "/home/svartalf/projects/dj-module-loader-
bug/.env/local/lib/python2.7/site-packages/django/utils/autoreload.py",
line 94, in gen_filenames
module_values = set(sys.modules.values())
TypeError: unhashable type: 'weakproxy'
}}}
I had reproduced this bug with a clean project and following environment:
{{{
Python 2.7.6
$ pip freeze
Django==1.8.5
Jinja2==2.7.3
MarkupSafe==0.23
argparse==1.2.1
wsgiref==0.1.2
}}}
Template settings:
{{{
template_loader = jinja2.ModuleLoader('compiled_templates')
TEMPLATES = [
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',
'DIRS': [os.path.join(BASE_DIR, 'templates')],
'APP_DIRS': True,
'OPTIONS': {
'loader': template_loader,
},
},
]
}}}
P.S. Not sure if it is an "Utilities" or a "Template system" component.
--
Ticket URL: <https://code.djangoproject.com/ticket/25624>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:1>
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hackedd):
* Attachment "25624.diff" added.
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hackedd):
This patch fixes the issue by ignoring modules without a `__file__`
attribute altogether. There are probably cases where this is not quite
desired behaviour, but I have no idea how to test this.
The other two options that I can think of to fix this, checking for
`weakref.ProxyType` or the `_jinja2_module_templates_` module name prefix,
don't really seem like the right solution either...
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:2>
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
@hackerdd That fix looks fine to me. I don't think it should actually
change the behavior, since modules without `__file__` were excluded later
on anyway. There's nothing we can do in the autoreloader with a module
that doesn't have a `__file__`.
I think this should be testable without too much difficulty, just by
inserting a problematic entry into `sys.modules` (in the test - and make
sure it gets removed after the test, using a `finally` block) and then
calling the `gen_filenames` function.
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:3>
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Indeed there are a few tests for `gen_filenames()` in
`tests/utils_tests/test_autoreload.py`. Could you try to add one as Carl
says?
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:4>
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mscout1):
What version of django will the officially be fixed in? I ask because I
have 1.8.9 and it still has this problem, and there is no 'target version'
or 'fix version' on this site.
(I apologize if this is not an appropriate place to ask this question, but
it is the only place I can find any information at all about this problem.
)
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:5>
Django
Reporter: svartalf | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: 1.8
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
There isn't a targeted version for the fix. It will be fixed when we have
a patch (with a test) to review.
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:6>
Django
Reporter: SvartalF | Owner: Tom
| Forbes
Type: Bug | Status: assigned
Component: Utilities | Version: 1.8
Severity: Normal | Resolution:
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tom Forbes):
* owner: nobody => Tom Forbes
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:7>
Django
Reporter: SvartalF | Owner: Tom
| Forbes
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tom Forbes):
* has_patch: 0 => 1
Comment:
Patch: https://github.com/django/django/pull/10870
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:8>
Django
Reporter: SvartalF | Owner: Tom
| Forbes
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom Forbes):
PR: https://github.com/django/django/pull/10872
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:9>
Django
Reporter: SvartalF | Owner: Tom
| Forbes
Component: Utilities | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"1e92407f83ed35be35f876777935b983ab9587be" 1e92407]:
{{{
#!CommitTicketReference repository=""
revision="1e92407f83ed35be35f876777935b983ab9587be"
Fixed #25624 -- Fixed autoreload crash with jinja2.ModuleLoader.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:9>
Django
Reporter: SvartalF | Owner: Tom
| Forbes
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: autoreload, jinja2, | Triage Stage: Accepted
ModuleLoader |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"728358c5cf96898a00eea2f81a2e305c0754f0c3" 728358c5]:
{{{
#!CommitTicketReference repository=""
revision="728358c5cf96898a00eea2f81a2e305c0754f0c3"
[2.2.x] Fixed #25624 -- Fixed autoreload crash with jinja2.ModuleLoader.
Backport of 1e92407f83ed35be35f876777935b983ab9587be from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25624#comment:10>