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

Catch SIGINT (Ctrl-c) and exit cleanly #75

Merged
merged 1 commit into from
May 16, 2017

Conversation

luke-orden
Copy link
Collaborator

I am adding signal to catch SIGINT in cli.py and not send it to the
child processes. When a SIGINT is caught, cli then calls the
stop_engine funcion in napalm_log, which sends a SIGTERM to each
child process. The child process will catch this SIGTERM and run it's
own stop function, while will close all pipes / sockets, and terminate
the loop.

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool stuff, just needs some more corners to be polished

if self.__up is False:
break
else:
raise
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let's raise and catch later the NapalmLogsExit exception (https://github.com/napalm-automation/napalm-logs/blob/master/napalm_logs/exceptions.py#L47) -- unused yet, but defined exactly for this scope :)

@@ -124,11 +129,18 @@ def start(self):
except ssl.SSLError:
log.exception('SSL error', exc_info=True)
continue
except socket.error:
if self.__up is False:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please raise or return

msg_dict, address = self.pipe.recv()
except IOError:
if self.__up is False:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments here: return and/or raise NapalmLogsExit

msg, address = self.pipe.recv()
except IOError:
if self.__up is False:
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here please

I am adding signal to catch `SIGINT` in `cli.py` and not send it to the
child processes. When a `SIGINT` is caught, `cli` then calls the
`stop_engine` funcion in `napalm_log`, which sends a SIGTERM to each
child process. The child process will catch this `SIGTERM` and run it's
own `stop` function, while will close all pipes / sockets, and terminate
the loop.
@luke-orden
Copy link
Collaborator Author

Addressed all comments and updated.

@mirceaulinic mirceaulinic merged commit 62d6c1f into napalm-automation:master May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants