jupyter-server-proxy icon indicating copy to clipboard operation
jupyter-server-proxy copied to clipboard

Fix SupervisedProcess on Windows

Open oeway opened this issue 5 years ago • 6 comments

This PR fixes https://github.com/jupyterhub/jupyter-server-proxy/issues/147 by running another even loop (ProactorEventLoop) which support subprocess on Windows.

(Tested on Windows 10)

oeway avatar Mar 24 '20 23:03 oeway

Thanks for investigating this. Do you have any experience of using Travis on Windows? https://docs.travis-ci.com/user/reference/windows/

I'm worried this could be inadvertently broken in future since most people develop and test on Linux or OS X.

No, but I just did a quick search and found travis does not support python on windows or OSX:

Python builds are not available on the macOS and Windows environments.

https://docs.travis-ci.com/user/languages/python/

Would it be ok to switch to Github Actions? All 3 major OS are supported, I can potentially give a quick try.

oeway avatar Mar 26 '20 13:03 oeway

@manics Made a PR for Github Actions which supports windows: https://github.com/jupyterhub/jupyter-server-proxy/pull/184

oeway avatar Mar 26 '20 13:03 oeway

Thank you very much for opening this PR! Windows support is absolutely necessary for many many tools these days, and while I don't have access to a machine, am happy there is work being done here!

However, simpervisor is fairly Linux / OS X specific - particularly, it relies on catching SIGCHLD signal to notice when the spawned process has died, and restart it. I'm not sure what a similar mechanism in Windows, but the most likely way forward is to implement that in Simpervisor (or a similar library), and use that here. I recognize that is way more work, but that's probably the way forward - mocking out signal handlers mean simpervisor can't do its job.

Thank you! I am happy that you commented here.

Indeed, this PR is more about make the subprocess run on windows, but I didn't try to test or fix the supervision part.

Since there is no POSIX signal on Windows, as you said, the supervision won't work. And we may need completely different implementation for the supervision.

oeway avatar Mar 26 '20 14:03 oeway

I did some search and couldn't find a solution for making sure process on windows are not dead. Well perhaps we don't need simpervisor if we already start the subprocess in a different thread? We can simply wait for the process to stop and restart again:

while True:
    proc = SupervisedProcess(...)
    await proc.start()
    is_ready = await proc.ready()
    if not is_ready:
        await proc.kill()
        raise web.HTTPError(500, 'could not start {} in time'.format(self.name))
    process.wait() # this will block the process until it stops

What do you think? @yuvipanda

oeway avatar Mar 26 '20 15:03 oeway

A gentle ping to @yuvipanda : Could you please let me know what you think about how to support windows?

oeway avatar May 06 '20 07:05 oeway

FYI: Since this PR is suspended, I had to publish the patched version for windows on pypi: https://pypi.org/project/jupyter-server-proxy-windows/ . It's a big urgly, but if you want a quick solution for windows users, do pip install jupyter-server-proxy-windows.

oeway avatar Jun 24 '20 14:06 oeway

Thank you for pionieering work towards supporting windows @oeway!!!

Support for windows has now been added by introducing it in Simpervisor (https://github.com/jupyterhub/simpervisor/pull/26), allowing this project to not do much besides start testing against Windows as I understand it (#392).

consideRatio avatar Jun 19 '23 12:06 consideRatio