Opened 16 years ago

Closed 11 years ago

#8413 closed Bug (duplicate)

Django runserver doesn't reload itself after updating files on disk that raised error on import.

Reported by: Yuri Baburov Owned by: Yuri Baburov
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: autoreload reload import error fix runserver
Cc: Leo Soto M. Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When autoreload is on,

  1. after any occasional error in any models.py file, reloading doesn't work for further updates of that file.
  2. modules that had import errors and updated on disk, are never reloaded.

Applied patch fixes the described problems and also allows to track various wrong usages of import API.

Attachments (2)

8413.patch (3.2 KB ) - added by Yuri Baburov 16 years ago.
Fix
8413_2.patch (2.1 KB ) - added by Yuri Baburov 16 years ago.
Better fix working through PEP 302

Download all attachments as: .zip

Change History (22)

by Yuri Baburov, 16 years ago

Attachment: 8413.patch added

Fix

comment:1 by Malcolm Tredinnick, 16 years ago

milestone: 1.0 maybepost-1.0
Triage Stage: UnreviewedDesign decision needed

I'm -1 on this because changing the behaviour of __import__ for everything just for a couple of edge-cases with the auto-reloader doesn't seem worth it. But we can make an ultimate decision after 1.0.

comment:2 by Yuri Baburov, 16 years ago

Owner: changed from burchik@… to Yuri Baburov

The patch will be rewritten to new hooks from PEP 302 today, it won't include import overriding.
Please don't judge without knowing all facts.

comment:3 by Yuri Baburov, 16 years ago

Well, actually, you were absolutely correct in previous comment. Adding new hook that doesn't deal with import in any way, so it's safe to use.

by Yuri Baburov, 16 years ago

Attachment: 8413_2.patch added

Better fix working through PEP 302

comment:4 by Leo Soto M., 16 years ago

Cc: Leo Soto M. added

comment:5 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 by Gabriel Hurley, 13 years ago

Component: django-admin.py runserverCore (Management commands)

comment:8 by Luke Plant, 13 years ago

Severity: Normal
Type: Bug

comment:9 by Aymeric Augustin, 12 years ago

Easy pickings: unset
Patch needs improvement: set
Triage Stage: Design decision neededAccepted
UI/UX: unset

Accepting the patch because the submitter addressed Malcolm's comment.

Unfortunately the GitHub link is broken, and the previous patch needs updating — at least, we no longer need Python 2.3 compatibility.

comment:10 by Aymeric Augustin, 12 years ago

This problem was also reported in #9589. This ticket has patches but their approach was rejected by a core dev and a BDFL.

in reply to:  10 comment:11 by Carl Meyer, 12 years ago

Replying to aaugustin:

This problem was also reported in #9589. This ticket has patches but their approach was rejected by a core dev and a BDFL.

I don't think that's accurate - the initial patch there was rejected (for good reason), but there are two subsequent patches that take quite a different (and, at least at first glance, much better) approach; there's no evidence those patches were ever given review. I think the second two patches on #9589 deserve another look if someone does look into fixing this.

Last edited 12 years ago by Carl Meyer (previous) (diff)

comment:12 by Aymeric Augustin, 12 years ago

Indeed, Carl is right. Whoever tackles this should review both tickets.

comment:14 by tdhutt@…, 11 years ago

This is really annoying. I briefly tested ebas's patch and it seems to work. Can somebody please apply it?

comment:15 by tdhutt@…, 11 years ago

PS: Here is the patch, since apparently github doesn't let you download them. Tested on django 1.4.1. The relavent file is in (usually) in /usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py so you can apply it like this:

cd /usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py
sudo patch autoreload.py ~/autoreload_patch.diff

where autoreload_patch.diff contains the following:

53a54

_error_files = []

57c58
< for filename in filter(lambda v: v, map(lambda m: getattr(m, "file", None), sys.modules.values())):
---

for filename in filter(lambda v: v, map(lambda m: getattr(m, "file", None), sys.modules.values())) + _error_files:

72a74,76

try:

del _error_files[_error_files.index(filename)]

except ValueError: pass

75a80,92

def check_errors(fn):

def wrapper(*args, kwargs):

try:

fn(*args, kwargs)

except (ImportError, IndentationError, NameError,

SyntaxError, TypeError), msg:

et, ev, tb = sys.exc_info()
if ev.filename not in _error_files:

_error_files.append(ev.filename)

raise

return wrapper

144,145c161,162
< reloader(main_func, args, kwargs)
<
---

wrapped_main_func = check_errors(main_func)
reloader(wrapped_main_func, args, kwargs)

comment:16 by anonymous, 11 years ago

Damnit. Try again:

53a54
> _error_files = []
57c58
<     for filename in filter(lambda v: v, map(lambda m: getattr(m, "__file__", None), sys.modules.values())):
---
>     for filename in filter(lambda v: v, map(lambda m: getattr(m, "__file__", None), sys.modules.values())) + _error_files:
72a74,76
>             try:
>                 del _error_files[_error_files.index(filename)]
>             except ValueError: pass
75a80,92
> def check_errors(fn):
>     def wrapper(*args, **kwargs):
>         try:
>             fn(*args, **kwargs)
>         except (ImportError, IndentationError, NameError, 
>                 SyntaxError, TypeError), msg:
>             et, ev, tb = sys.exc_info()
>             if ev.filename not in _error_files:
>                 _error_files.append(ev.filename)
> 
>             raise
>     return wrapper
> 
144,145c161,162
<     reloader(main_func, args, kwargs)
< 
---
>     wrapped_main_func = check_errors(main_func)
>     reloader(wrapped_main_func, args, kwargs)

comment:17 by tdhutt@…, 11 years ago

After more extensive testing, it's working quite well but I did get this one error:

Unhandled exception in thread started by <function wrapper at 0x1f6e8c0>
Error in sys.excepthook:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/apport_python_hook.py", line 50, in apport_excepthook
    if not enabled():
TypeError: 'NoneType' object is not callable

Original exception was:
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py", line 83, in wrapper
    fn(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/commands/runserver.py", line 111, in inner_run
    ipv6=self.use_ipv6, threading=threading)
  File "/usr/local/lib/python2.7/dist-packages/django/core/servers/basehttp.py", line 253, in run
    httpd.serve_forever()
  File "/usr/lib/python2.7/SocketServer.py", line 225, in serve_forever
    r, w, e = select.select([self], [], [], poll_interval)
AttributeError: 'NoneType' object has no attribute 'select'

comment:18 by Ramiro Morales, 11 years ago

Keywords: runserver added
Summary: Django doesn't reload itself after updating files on disk that raised error on import.Django runserver doesn't reload itself after updating files on disk that raised error on import.

comment:19 by Ramiro Morales, 11 years ago

Patch needs improvement: unset

I've updated the patch contributed by berto to #9589: https://github.com/ramiro/django/compare/8413_9589_reload_runserver

Testing of the patch on Unixy platforms (including Mac OS X) is welcome. I will try to test it on Windows.

I plan to commit this to master in a few days.

comment:20 by Tim Graham, 11 years ago

Resolution: duplicate
Status: newclosed

Closing as duplicate of #9589 - it looks like the fix was committed with a reference to that ticket but not this one so it wasn't closed automatically.

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