search

jupyterhub / simpervisor

Simple Python3 Supervisor library
BSD 3-Clause "New" or "Revised" License
12 stars 10 forks source link

Register existing signal handlers in simpervisor's atexit module #39

Open mahendrapaipuri opened 1 year ago

mahendrapaipuri commented 1 year ago

Effectively, we are checking if the existing handlers are different from the default ones and if they are not default, we execute these handlers in the signal handler. I am not sure if it is the most elegant solution but happy to get any insights!!

Fixes #38

welcome[bot] commented 1 year ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

consideRatio commented 9 months ago

Thank you for working this @mahendrapaipuri and sorry for the slow turnaround of review. I'm not confident this is right yet.

I pushed few commits, but is this working correctly etc? I've not added a test, which i think would be good to have. I also don't feel confident on the windows parts here.

manics commented 9 months ago

I've got access to a Windows VM with conda. It's not clear how to test this specific functionality though.

mahendrapaipuri commented 9 months ago

@consideRatio Thanks a lot for looking into this PR. I still need to look into changes you have pushed. I dont have a lot of experience with windows and neither I have access to windows machine to test it.

@manics If you have access to windows machine, we can test this as described in the issue. Do you think it is doable?

mahendrapaipuri commented 9 months ago

@consideRatio @manics I have pushed a unit test to check if we are calling existing non default handlers. Please have a look at it when you can. Not sure if we can test this functionality on windows easily though!