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,
- after any occasional error in any models.py file, reloading doesn't work for further updates of that file.
- 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)
Change History (22)
by , 16 years ago
Attachment: | 8413.patch added |
---|
comment:1 by , 16 years ago
milestone: | 1.0 maybe → post-1.0 |
---|---|
Triage Stage: | Unreviewed → Design 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 , 16 years ago
Owner: | changed from | to
---|
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 , 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.
comment:4 by , 16 years ago
Cc: | added |
---|
comment:6 by , 15 years ago
comment:7 by , 14 years ago
Component: | django-admin.py runserver → Core (Management commands) |
---|
comment:8 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:9 by , 13 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
Triage Stage: | Design decision needed → Accepted |
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.
follow-up: 11 comment:10 by , 13 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.
comment:11 by , 13 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 #9859 deserve another look if someone does look into fixing this.
comment:12 by , 13 years ago
Indeed, Carl is right. Whoever tackles this should review both tickets.
comment:13 by , 12 years ago
After reviewing all the patches, i came up with this patch against the 1.4.x branch:
https://github.com/ebas/django/commit/970cee879c84eb495f385822e4e9c99f2aed2088
Its based on a patch found in this post:
https://groups.google.com/forum/?fromgroups=#!topic/django-developers/rJkUEGdMGV0
The patch consists of the top commits on this branch:
https://github.com/rca/django/commits/master/django/utils/autoreload.py
Just to be complete, the noteworthy other patches from both #9589 and #8413:
comment:14 by , 12 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 , 12 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 , 12 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 , 12 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 , 12 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 , 12 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 , 11 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
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.
Fix