Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto_reload support for Windows #1346

Closed
mungojam opened this issue Oct 5, 2018 · 6 comments · Fixed by #2499
Closed

auto_reload support for Windows #1346

mungojam opened this issue Oct 5, 2018 · 6 comments · Fixed by #2499

Comments

@mungojam
Copy link

mungojam commented Oct 5, 2018

This would be really useful for us. We don't have an option to use Linux for development unfortunately

@vltr
Copy link
Member

vltr commented Oct 5, 2018

Today our reloader is completely dependant in the way POSIX works ... But I really think that a more generic approach is possible to make this behavior available for POSIX and Windows almost transparently, but I'll have to do some hard digging in there to see if my idea really works ...

@abuckenheimer
Copy link
Contributor

I took a quick look was surprised to find that the re-loader is only really dependent on posix for killing the existing server process and children and not to detect file changes. My guess is we could probably use something like psutil to make this more uniformly platform independent but not sure we'd want that as a dependency.

@yunstanford
Copy link
Member

This functionality could actually be put into a plugin. And current implementation is also a little bit error-prone. It'll certainly require more mature process management.

@vltr
Copy link
Member

vltr commented Oct 9, 2018

@abuckenheimer @yunstanford I know I have made some tools in the past that were way more complex because they required either binary or byte-compiled code to be hot-replaced, either in Windows or Linux or (whatever the name of that embed machine made in hell). This should not be complicated, with my first idea being wrapping the server stuff and launching a second process with multiprocessing.spawn, that would certainly work on Linux and Windows. The main process would watch for file changes and restart (kill+spawn) the new server on some events (either by using inotify in Linux or simple polling in Windows, though I think there might be something better / already done for that).

Also, I agree with the idea that this should be put into a plugin and not within Sanic since it is just a tool, not a production specific feature.

Please, let's continue this discussion on the community forums if possible 😉

@stale
Copy link

stale bot commented May 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@stale stale bot added the stale label May 14, 2019
@andreymal
Copy link
Contributor

@​stale nope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants