gunicorn
gunicorn copied to clipboard
Watchdog based reloader
Replace the reloader with a reloader based on the watchdog package. This new reloader should work on all the platforms. Most popular platforms use an underlying event system like inotify or kqueue, but watchdog will fall back to a polling implementation.
Start the reloader after loading the WSGI entry point of the application so that the reloader picks up the application code modules when scanning sys.modules and have the worker send SIGQUIT to itself in its callback because that seems to make exits happen more quickly, which is more in line with the common development use case for reloading.
There are probably some bits I missed, documentation and whatnot, but I wanted to get this out here for feedback.
Closes #1477 Closes #1537
Might also fix other problems that have been reported with the reloader, like #1494 and #1562.
Indeed :-). Fixed.
@tilgovi, will this work solve #1494?
I don't know, but I would love to find out! Please comment if you test it.
This sounds nice :)
I tried to address all the comments. Please weigh in on making watchdog required or anything else. Thanks!
Hey @tilgovi, would you mind rebasing since coverage reports should be fixed in master?
Updated the branch:
- rebased onto latest master
- changed
ImportErrortoRuntimeError - removed all changes other than to the reloader instantiation
I think it's cleaner to have the worker send itself SIGQUIT because then exit is handled from the main thread rather than a reloader thread, but I'll make that a separate PR.
I think the reloader should be started after loading the wsgi callable to ensure as much application code as possible is loaded, but I'll make that a separate PR.
For a 19.x release, I would rather leave 'inotify' as an alias for 'auto' so that there is no breaking change at all. For R20, we can make it very explicit if we want to, depending on how we vendor watchdog or some of its modules.
@Code0x58 if you could point me to an issue for the directory/filename mistake, I would appreciate that.
where is my comment on that PR? I didn't have any answer on it :(
@tilgovi did you have any answer from the maintainer of the watchdog module?
I also still want addressed the question of support and how core the reload feature should be. Should it be discussed in a separate ticket?
+1 for getting this finished/merged. @tilgovi Are you still up for finishing it?
@philfreo I am still up for it, but would be happy to let anyone else take it to completion, as well. Are you interested?
I think we still have a couple open questions:
- Watchdog seems unmaintained. Do we want it as a dependency?
- Should this be core or extra functionality (either via package
extrasor a separate package)?
Please do handle it then - you'd be much better than I :) Just glad to see it moving forward.
My 2c: if the reload option is part of core, then it seems like this should be too?
The pylons project recently replaced its thread-based reloader with hupper [1] which does a more robust reloading using either polling or watchdog (if installed). I'd be happy to help with issues if anyone made an attempt to integrate it here.
[1] https://docs.pylonsproject.org/projects/hupper/en/latest/
I would love to replace this with hupper. I think hupper is a better dependency than watchdog itself. We have managed not to introduce any out-of-tree dependencies in Gunicorn so far, but maybe @benoitc would consider a dependency on hupper. It could even be optional.
go ahead i missed that.
Let's revisit after #1791 and try with hupper.
Actually, not sure #1791 is still applicable. I will check.
what happened to that PR ? Is this still something we can work on @tilgovi ?
I think I was going to try to use hupper instead of watchdog, if it's acceptable to add a dependency. Maybe make it optional? What do you think?
My 2 cents:
- Any reloading dependency should be made optional as an extra requirement. On runtime, silently downgrade the reloader implementation if some import is missing. Maybe raise a warning?
- Watchdog is a rather ubiquitous package. I've seen it integrated with werkzeug/flask, django and other development servers. Everybody is happy with it and its a stable component from the python ecosystem so... I do not understand the reluctance to introduce it.
- AFAIU
hupper's purpose is to totally replace any reloading hassle from other python programs. So, without further ado: today, right now, with any past gunicorn version, without any integration in gunicorn's side, anyone could runhupper -m gunicorn application:appinstead ofgunicorn --reload application:app. There would be nothing to implement in our side but dropping the--reloadoption and advising people topip install hupperinstead. All the complexity about choosing the best reloader for a given platform depending on the presence/absence of watchdog or anything else would be just moved away without having to integrate anything. Yet, this is a much more marginal project than watchdog so I can't really vouch for this (out of unfamiliarity with hupper).
P.S: behind the scenes hupper will use watchdog if it can.