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

Restrict the child process received interrupt signal. #84

Merged
merged 2 commits into from
Oct 16, 2014

Conversation

tonyseek
Copy link
Contributor

Hi,

This PR change the behavior of honcho run to restrict the child process received interrupt signal. It can prevent the Ctrl - C caused crash in honcho run ipython and honcho run python manage.py shell.

Please review it. Thanks.

@slafs
Copy link
Collaborator

slafs commented Oct 11, 2014

This looks very nice! Much cleaner solution than we were trying to have in #70. Maybe we should wait a bit for any comment from @nickstenning or @claymation before I merge it.

@claymation
Copy link

I wouldn't say it's cleaner than #70—it's the same solution: ignore SIGINT in honcho run.

But yeah, let's merge it and finally get this fixed :)

👍

@claymation
Copy link

(It would be nice to follow the guidelines in Proper Handling of SIGINT/SIGQUIT, but I suppose that can come later).

@slafs
Copy link
Collaborator

slafs commented Oct 16, 2014

Yeah sorry @claymation. Actually I was thinking about my poor approach that I mentioned in #70. I guess I'll merge it then.

slafs added a commit that referenced this pull request Oct 16, 2014
Restrict the child process received interrupt signal. closes #70 #79
@slafs slafs merged commit 000a7e6 into nickstenning:master Oct 16, 2014
@tonyseek
Copy link
Contributor Author

Thank you :-)

@tonyseek tonyseek deleted the subprocess-interrupt branch October 16, 2014 10:25
@slafs
Copy link
Collaborator

slafs commented Oct 16, 2014

No. Thank you :). @claymation can you please open a new issue for "Proper Handling of SIGINT/SIGQUIT" that you mentioned?

@claymation
Copy link

Sure, #85.

Thanks for merging this, now I can stop maintaing our own fork :)

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

Successfully merging this pull request may close these issues.

3 participants