Skip to content
This repository has been archived by the owner on Jan 16, 2020. It is now read-only.

Added handling for SIGTERM as well as SIGINT #56

Closed
wants to merge 1 commit into from
Closed

Added handling for SIGTERM as well as SIGINT #56

wants to merge 1 commit into from

Conversation

dnschnur
Copy link
Contributor

A quick change to resolve (if I understand correctly) #54; just pulled out the handler and registered it on SIGTERM as well as SIGINT.

@gnarf
Copy link
Contributor

gnarf commented Oct 21, 2012

@scottgonzalez can you do some quick testing and merge/tag this?

@scottgonzalez
Copy link
Contributor

Thanks @dnschnur, but @gnarf37 and I just discussed this and we're going to use HUP to restart everything, including the manager.

@dnschnur
Copy link
Contributor Author

@scottgonzalez SIGINT and SIGTERM don't trigger a restart; they're shutdown requests. Regardless of what other signals you handle, and what they're used for, these two should lead to a graceful shutdown.

@scottgonzalez
Copy link
Contributor

SIGINT will result in a graceful shutdown, SIGHUP will result in a graceful restart. We don't plan on doing anything specific for SIGTERM, should we?

@dnschnur
Copy link
Contributor Author

SIGTERM is (roughly speaking) the equivalent of SIGINT, but coming from the system instead of the user. So it's normally a good idea to catch it, and handle it in the same way; I believe that's what #54 was asking for.

@scottgonzalez
Copy link
Contributor

Ok, I'll reopen this and land it after I finish the SIGHUP addition. #54 was written because @gnarf37 wanted to use the default signal from puppet, but he decided we should be sending SIGHUP instead. I don't really know anything about signals, so I'm just going on what others tell me, so thanks for staying on top of this.

@scottgonzalez
Copy link
Contributor

@dnschnur Do you want to update this to work with the new code?

@dnschnur
Copy link
Contributor Author

Sure; I should have time either this evening or tomorrow morning.

@scottgonzalez
Copy link
Contributor

Thanks.

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

Successfully merging this pull request may close these issues.

3 participants