[Django] #29044: Changes to autoreload to make atexit calls work in the child process.
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: nobody
Orange |
Type: New | Status: new
feature |
Component: Core | Version: 2.0
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently the child process is killed if the parent is interrupted (i.e.
CTRL+C). This means in turn that atexit handlers are not called. This
traces to the use of subprocess.call which kills the child process if it
catches an exception (KeyboardInterrupt for instance). This is fixed if we
use Popen instead (which is what call does) and instead of killing the
child process, simply wait for it to complete. My fix can been seen here:
https://github.com/CrazyCasta/django/commit/cfbb6e2f5e95733b8a2448cef18227e56d6e37b5
but I need some help.
Two things:
1. Feedback on how best to handle this. It is my opinion that the best way
to handle this is to the child process finish itself up however long that
might take and then eventually exit. If there is some sort of bug in the
code (users' or Django's) that prevents this from the child process from
completing then the bug should be fixed. Another way to handle this would
be to timeout the p.wait in the exception with some configurable delay and
then kill and wait.
2. How should this be tested. I'm guessing that the best way to do this
would be to create a very simple command, create a subprocess that calls
that command, wait for the grand-child process to startup, send the child
process an interrupt and wait for the grand-child process to complete. It
appears that the current code avoids creating any child processes. Is
there a wish not to create child processes in the tests, or would it be ok
to do so as long as it completes quickly (i.e. don't use runserver, create
a dummy command that will run much faster).
--
Ticket URL: <https://code.djangoproject.com/ticket/29044>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Core (Management | Version: 2.0
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
Comment:
Setting needs tests flag. As I mentioned above, I need some feedback on
what the proper way of testing this would be.
--
Ticket URL: <https://code.djangoproject.com/ticket/29044#comment:1>
Django
-------------------------------------+-------------------------------------
Component: Core (Management | Version: 2.0
commands) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
We are trying to change completely the development webserver autoreloader.
See #27685 and the related [https://github.com/django/django/pull/8819
PR], so I suspect no further features will be added to the current code.
We are finding the same problem there as you: How to test this code in
presence uf multiple processes, timing constraints, etc.? If you can
contribute somehow to that effort e.g. by testing your use case and with
ideas/code about how to implement the tests it will be greatly
appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/29044#comment:2>