pathbird / poetry-kernel

Python Jupyter kernel using Poetry for reproducible notebooks
MIT License
246 stars 7 forks source link

Windows Does not have SIGKILL #3

Closed amirhessam88 closed 2 years ago

amirhessam88 commented 2 years ago

I believe the following line needs more attention to be compatible with Windows considering windows does not have SIGKILL: https://github.com/pathbird/poetry-kernel/blob/main/poetry_kernel/__main__.py#L39

twavv commented 2 years ago

Does this actually cause any issues? Is SIGKILL not defined on Windows?

amirhessam88 commented 2 years ago

Yeah; I am personally a Mac user I could easily use poetry kernel for any projects and use it in Jupyter Lab. However, I tried to do the same thing for a friend of mine who is a Windows user, and I could not. I ended up adding the kernel manually as shown here; The error I was getting was related to SIGKILL that could not be found on Windows. Based on this Link, In Windows, signal() can only be called SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, or SIGTERM. ValueError will raise A in other cases.

twavv commented 2 years ago

Happy to accept a PR that has separate logic for defining a different forward_signals set for Windows versus Mac/Linux.

Based on the code, it's not actually the SIGKILL causing the issue (we actually remove SIGKILL from the set of signals under consideration for forwarding). Instead we should just define the signal set to be the ones you suggested above (on Windows at least).

twavv commented 2 years ago

Thanks! Closed in #4 and released as v0.1.2.