[Django] #24704: Development server do not restart on SynaxError
Django
--------------------------------------+--------------------
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
Django documentation states:
> '''each time you change Python code''' while the server is running, the
server will check your entire Django project for errors (see the check
command). If any errors are found, they will be printed to standard
output, '''but it won’t stop the server'''.
https://docs.djangoproject.com/en/1.7/ref/django-admin/#runserver-port-or-
address-port
However in my case the server process finishes with exit code 1 when
syntax error appears in admin.py or models.py. Below is the error message
that I see in my console.
{{{
File "/home/.../src/core/admin.py", line 16
foo =
^
SyntaxError: invalid syntax
}}}
So it seems there is either a bug or a mistake in the documentation.
Django 1.7.5
Python 2.7.6
virtualenv 1.11.4
Ubuntu 14.04.2 LTS
--
Ticket URL: <https://code.djangoproject.com/ticket/24704>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* needs_docs: => 0
* resolution: => invalid
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
A `SyntaxError` is a Python concept, not something relevant to the check
command. When writing the docs, I'd like to assume enough familiarity with
Python that we don't need to explain that a program can't run if it
contains syntax errors. Hope that explanation helps.
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:1>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by artemrizhov):
* status: closed => new
* resolution: invalid =>
Comment:
Thank you for your explanation. To be honest, it does not help at all. The
question is not about the program, but about the dev server which hosts
it.
Please note that the server does not exit when syntax error happens in
views.py. So, it really makes sense to explain how you did that impossible
thing! ;) I'm joking, please don't waste your time for explanation of
this. But please believe many perfectionists have a lot of another work
and do not have time to dive into the Django core.
The server and the application are two different entities. I don't think
this should be clear that the server can not handle syntax errors in the
app. Furthermore, '''it does''' in some cases. And even furthermore, I'm
sure it is possible to implement such feature for any cases, even with
Python, for example, with a master process which is watching and rerunning
the app when it exits with error.
When I read documentation I'd want to believe that the written text states
true. If not, why not to add a note about this? I found few tickets about
this problem both fixed and rejected. So I came to the documents while
searching for the truth. But it lies :) Just replace "any errors" with
something more honest. I'd not spend time to write this ticket if the docs
contains notes about this, and you'd not spend time to answer to my
question. And I'm sure many people will thank you.
But it would be really better if Django dev server can handle any errors.
I've started learning this issue because I've made too many manual
restarts of the dev server while working on site admin area.
The simplest workaround is `while true; do ./manage.py runserver ; sleep 1
; done`. But it does not work well with PyCharm which I really love.
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:2>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by artemrizhov):
Maybe I've selected wrong component for this issue? Does it make sense to
change it from "Core (System checks)" to "Management commands" or "Other"?
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:3>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "24704.diff" added.
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
Does the attached patch help?
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:4>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by artemrizhov):
Yes, this version seems much more clear! Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:5>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"0f2e82b9ec9797147945a6f9a402b5ae0fb1d9f4" 0f2e82b9]:
{{{
#!CommitTicketReference repository=""
revision="0f2e82b9ec9797147945a6f9a402b5ae0fb1d9f4"
Fixed #24704 -- Clarified system check interaction with runserver.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:6>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Severity: Normal | Resolution: fixed
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8fa763a983608cce7567e3189cf8fec02f8f0bd0" 8fa763a]:
{{{
#!CommitTicketReference repository=""
revision="8fa763a983608cce7567e3189cf8fec02f8f0bd0"
[1.8.x] Fixed #24704 -- Clarified system check interaction with runserver.
Backport of 0f2e82b9ec9797147945a6f9a402b5ae0fb1d9f4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:7>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Severity: Normal | Resolution: fixed
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"f6ecb5407e5e7bb11e8d1a36413f20eebc39e0af" f6ecb540]:
{{{
#!CommitTicketReference repository=""
revision="f6ecb5407e5e7bb11e8d1a36413f20eebc39e0af"
[1.7.x] Fixed #24704 -- Clarified system check interaction with runserver.
Backport of 0f2e82b9ec9797147945a6f9a402b5ae0fb1d9f4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:8>
Django
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Severity: Normal | Resolution: fixed
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by artemrizhov):
* cc: artem.rizhov@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: 1.7
checks) |
Severity: Normal | Resolution: fixed
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: nobody
Type: Bug | Status: new
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* component: Core (System checks) => Core (Management commands)
* has_patch: 1 => 0
* resolution: fixed =>
* stage: Unreviewed => Accepted
Comment:
It seems that before 0d2c8ff2be733c7cc83a023bbafe0258faa5603c syntax
errors in models.py didn't stop the server. Now it does:
{{{
Traceback (most recent call last):
File "manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/tim/code/django/django/core/management/__init__.py", line
428, in execute_from_command_line
utility.execute()
File "/home/tim/code/django/django/core/management/__init__.py", line
420, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/tim/code/django/django/core/management/__init__.py", line
288, in fetch_command
commands = get_commands()
File "/home/tim/code/django/django/core/management/__init__.py", line
125, in get_commands
apps.populate_models()
File "/home/tim/code/django/django/apps/registry.py", line 123, in
populate_models
app_config.import_models(all_models)
File "/home/tim/code/django/django/apps/base.py", line 164, in
import_models
self.models_module = import_module(models_module_name)
File
"/home/tim/.virtualenvs/django34/lib/python3.4/importlib/__init__.py",
line 109, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 2231, in _gcd_import
File "<frozen importlib._bootstrap>", line 2214, in _find_and_load
File "<frozen importlib._bootstrap>", line 2203, in
_find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 1200, in _load_unlocked
File "<frozen importlib._bootstrap>", line 1129, in _exec
File "<frozen importlib._bootstrap>", line 1444, in exec_module
File "<frozen importlib._bootstrap>", line 1549, in get_code
File "<frozen importlib._bootstrap>", line 1509, in source_to_code
File "<frozen importlib._bootstrap>", line 321, in
_call_with_frames_removed
File "/home/tim/code/mysite/polls/models.py", line 15
class MyModelmodels.Model):
}}}
Not sure if the old behavior can be restored, but reopening in case
someone would like to investigate.
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* owner: nobody => aaugustin
* status: new => assigned
Comment:
Aaargh.
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* has_patch: 0 => 1
Comment:
I had a look at this. Unfortunately our reloading technique is to watch
''imported'' Python files which makes things complicated (in addition to
requiring hacks for everything that isn't a Python file).
See [https://github.com/django/django/pull/5106 this PR] for a possible
"solution".
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I'm not proud of that PR but I think it's an improvement and it should go
in until we [https://groups.google.com/d/msg/django-
developers/voXNIDdDcpU/mnmr9NCPCgAJ reconsider the autoreloader's design].
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: 1 => 0
Comment:
Patch was improved.
Since it can be a real inconvenience in day-to-day development and it can
be considered a regression in 1.7, it would make sense to backport at
least to 1.8 (LTS). The patch doesn't apply cleanly but shouldn't be very
hard to backport.
If I want to backport, I have to add an entry in the release notes for
1.8.5, anything else?
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Just release notes are required for the backport.
Marking as "Patch needs improvement" as it seems the patch still doesn't
work if pyinotify is installed. It does work on Windows.
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: assigned
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Component: Core (Management | Version: 1.7
commands) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"b79fc11d730b5beff92e9dd8853a61524cdeffe3" b79fc11]:
{{{
#!CommitTicketReference repository=""
revision="b79fc11d730b5beff92e9dd8853a61524cdeffe3"
Made the autoreloader survive all exceptions.
Refs #24704.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"fe6ddb837d18bd4e71cd22fc18272d31478b19f2" fe6ddb8]:
{{{
#!CommitTicketReference repository=""
revision="fe6ddb837d18bd4e71cd22fc18272d31478b19f2"
Fixed #24704 -- Made the autoreloader survive SyntaxErrors.
With this change, it's expected to survive anything except errors
that make it impossible to import the settings. It's too complex
to fallback to a sensible behavior with a broken settings module.
Harcoding things about runserver in ManagementUtility.execute is
atrocious but it's the only way out of the chicken'n'egg problem:
the current implementation of the autoreloader primarily watches
imported Python modules -- and then a few other things that were
bolted on top of this design -- but we want it to kick in even if
the project contains import-time errors and django.setup() fails.
At some point we should throw away this code and replace it by an
off-the-shelf autoreloader that watches the working directory and
re-runs `django-admin runserver` whenever something changes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"cc14d51ee8325c82665cb98af4dfe49aab565d52" cc14d51]:
{{{
#!CommitTicketReference repository=""
revision="cc14d51ee8325c82665cb98af4dfe49aab565d52"
Fixed #24704 -- Made the autoreloader survive SyntaxErrors.
With this change, it's expected to survive anything except errors
that make it impossible to import the settings. It's too complex
to fallback to a sensible behavior with a broken settings module.
Harcoding things about runserver in ManagementUtility.execute is
atrocious but it's the only way out of the chicken'n'egg problem:
the current implementation of the autoreloader primarily watches
imported Python modules -- and then a few other things that were
bolted on top of this design -- but we want it to kick in even if
the project contains import-time errors and django.setup() fails.
At some point we should throw away this code and replace it by an
off-the-shelf autoreloader that watches the working directory and
re-runs `django-admin runserver` whenever something changes.
Backport of fe6ddb837d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: artemrizhov | Owner: aaugustin
Type: Bug | Status: closed
Component: Core (Management | Version: 1.7
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"2b08b364889d47502a121cd07c69937b94003904" 2b08b364]:
{{{
#!CommitTicketReference repository=""
revision="2b08b364889d47502a121cd07c69937b94003904"
[1.8.x] Made the autoreloader survive all exceptions.
Refs #24704.
Backport of b79fc11d73 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24704#comment:23>