Hello Tom, I think the PR is on the right track. Now it needs to be taken beyond the "proof of concept" stage.
Once you have code, docs, and if possibly some tests — currently there are few due to the difficulty of testing auto reloading, especially edge cases — for steps 1 and 2, I can review changes again and help you get it merged. Since this is a major overhaul, we'll have to think carefully about how the behavior changes and possible backwards incompatibilities. That said, the autoreloader is a dev tool. Changing it isn't going to break anyone's production. This makes the big-bang approach viable. Thanks for working on this! -- Aymeric. > On 29 Sep 2017, at 21:03, Tom Forbes <t...@tomforb.es> wrote: > > Hello, > I've been thinking on and off about how to improve the autoreloader > implementation and I wanted to gather some feedback on potential solutions. > > For some background, Django uses a fairly basic autoreload implementation > that simply polls the last modified time for loaded Python files once a > second. While this isn't the most efficient, it does work and has worked > quite well for a long time. When running manage.py runserver, the > autoreloader will launch a child "manage.py" with the same arguments and this > child process actually runs Django and serves requests. To reload, the child > process exits with exit code 3 and the parent restarts it. The code is some > of the oldest in Django, with a fair bit of it not touched in 9-12 years. > > While it works (and I'm a believer in "if it isn't broke don't fix it") there > are some architectural and performance issues with the current code: > > - Polling every second is not very efficient > - Detecting when the child process has exited during startup (i.e problem in > settings.py) is problematic and the code is rather nasty > - i18n files are 'reloaded' when they change in a rather hacky way (resetting > private attributes in another module) > - There is limited support for extending the current implementation, and > there are cases during development where the parent autoreloader will > terminate. > > I don't want this email to be too long, so I'm going to summarize what I > think would be a good approach to tackling these problems. > > 1. Refactor the current implementation by removing `pyinotify`, redundant > python 2 checks and implement a 'file_changed' signal so other parts of > Django can react to file changes (i.e the i18n module can handle resetting > it's own state). > 2. Add support for the "watchdog" library as a replacement for pyinotify. > Watchdog implements file system notifications for all major platforms using a > fairly simple API, so we can remove polling and have instant reloading. Also > support Watchman, a notification Daemon from Facebook. > 3. Add support for more advanced features, like proper handing of startup > errors and socket sharing. > > I've got a merge request that implements all three stages as a proof of > concept, but I think it's far too much a change to be done at once and should > be done carefully stage by stage. One and two are fairly simple to implement, > but three requires see careful consideration as to the best approach (this > message is long enough already, I don't want to describe them here). > > Does anyone have any feedback on these ideas? Is it worth persuing even if > the current implementation works ok-ish? > > > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to django-developers+unsubscr...@googlegroups.com > <mailto:django-developers+unsubscr...@googlegroups.com>. > To post to this group, send email to django-developers@googlegroups.com > <mailto:django-developers@googlegroups.com>. > Visit this group at https://groups.google.com/group/django-developers > <https://groups.google.com/group/django-developers>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CAFNZOJMT9qDk-4pKKXSJysEQCmd6CGxMZBYZs_7BQs_WbAqL6g%40mail.gmail.com > > <https://groups.google.com/d/msgid/django-developers/CAFNZOJMT9qDk-4pKKXSJysEQCmd6CGxMZBYZs_7BQs_WbAqL6g%40mail.gmail.com?utm_medium=email&utm_source=footer>. > For more options, visit https://groups.google.com/d/optout > <https://groups.google.com/d/optout>. -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/EF5E434E-96EC-4FF7-BC03-477010C8A472%40polytechnique.org. For more options, visit https://groups.google.com/d/optout.
