I’m getting back into contributing to Django after a long hiatus, and I figured I’d take a look at the autoreloader.
The tl;dr is that while Watchman is pretty OK, the pywatchman library seems to be a bit of a dumpster fire. It’s completely broken (see pywatchman raises SystemError on Python 3.10 · Issue #970 · facebook/watchman · GitHub) so anyone using 1.4.1 won’t benefit from any watchman related autoreloading despite watchman itself working fine. With the recent layoffs at Meta, and given that no release has been successfully made since 2017, I think that waiting for a release isn’t a great idea.
I see that we have two paths to take:
Rip out watchman and support something else
Replace the pywatchman client
I think that option 2 is the most viable because majority of the code in the pywatchman library is around the bser codec they support and some specific stuff around Windows IO.
I think that creating a small client using the JSON encoding scheme would be pretty simple. I’m going to create a proof of concept to see but I wanted to gather some feedback or suggestions. If the “build a simpler pywatchman client” is viable, there are questions about ownership and where the code should live. I’d propose a separate library under the Django organisation rather than integrating this code directly into Django.
As a side note, I’m looking forward to meeting some of you face-to-face in Edinburgh!
I’m not a fan of the pywatchman library - it seems overly complex and requires us to implement a number of additional things that it really should take care of.
One pretty core example is simply watching directories (roots). Watchman may be watching a parent directory which means watching a root may return a relativeroot. Unfortunately all events are propagated as if they came from the parentroot, which requires us to do extra housekeeping in order to match a relative subscription to a parent root.
The initial implementation was only about 130 lines or so but I went a bit further and implemented this housekeeping and a few convenience methods, so it now weighs in at ~340 lines. I also noticed a few deficiencies in my Django implementation that I fixed here (namely removing the need for sleeping, and there’s a small memory leak that’s hidden by the restarting).
Suggesting users install the library from git doesn’t seem like great advice even if main does currently work.
IMO shipping this ourselves could be a viable solution that wouldn’t be terribly complex to maintain, and would help the wider ecosystem.
Wow, that looks great. I never bothered installing pywatchman. Given how short it is, I am personally leaning towards adding it to Django itself (or at least adding it as dependency). I really would love to see a autoreload solution that does not drain battery more than needed where I do not have to remember to install something into every venv I want to use Django in (I understand that watchman still needs to be installed, but that is a one time thing).
If @orf or others would like to get it to the same level as the watchman integration, happy to help review and merge. I will hopefully have time to work on it later in the year. That could then be a tested base for adding support into Django.
I would definitely be up for using watchfiles, it looks quite good. Couple of thoughts:
It doesn’t seem to be possible to add/remove watches at runtime. The common use case for this is watching imported modules in site-packages (which change at runtime), I guess we’d need to create a single site packages watch and do some manual filtering.
But this does get a bit complex - if you have a virtual environment that is inside the project directory those will overlap. The thing I do like about watchman is that it hides this overlapping complexity from you, but it’s more than possible to implement ourselves.
Watchman does also have some nice features around delaying notifications during git operations. Again, we can definitely implement this ourselves or possibly add it upstream.
The other complexity is symlinks. I’m not entirely sure how watchman handles this, other than “I think it does and I think it’s transparent”. If it’s also transparent with watchfiles then I think it’s a viable replacement.