Agent: register signal handlers #1627
Conversation
Codecov Report
@@ Coverage Diff @@
## 1593-i-master #1627 +/- ##
=================================================
+ Coverage 44.08% 44.69% +0.61%
=================================================
Files 468 461 -7
Lines 13642 13414 -228
=================================================
- Hits 6014 5996 -18
+ Misses 7628 7418 -210
Continue to review full report at Codecov.
|
2674e87 to
bb5edbc
Compare
bb5edbc to
03e877d
Compare
| import win32api | ||
|
|
||
| signal.signal(signal.SIGBREAK, stop_signal_handler) | ||
| win32api.SetConsoleCtrlHandler(stop_signal_handler, True) |
There was a problem hiding this comment.
Based on this it has a 5s timeout. I think it's worth noting it in the code.
| def __init__(self, master: IMaster): | ||
| self._master = master | ||
|
|
||
| def __call__(self, signum, __=None): |
There was a problem hiding this comment.
what's the point of specifying the default argument for __? Also, no point in using double underscore, we should use _.
There was a problem hiding this comment.
As a guideline I suggest using _ for one unused argument and *_ if there are many
0401721 to
e00fd64
Compare
| def __init__(self, master: IMaster): | ||
| self._master = master | ||
|
|
||
| def __call__(self, signum, __=None): |
There was a problem hiding this comment.
We should be wary of using the __call__. It's meant to hide the state from the caller, which is unnecessary in this case. Generally, it's better to have
signal.signal(signal.SIGINT, stop_signal_handler.handle_signal)
because this way it's clear that signal.signal expects to get a function. As it is right now, it looks like it expects an object of class StopSignalHandler until you actually read the StopSignalHandler or signal.signal
Agent will now handle interrupt and break signals on linux and windows
The call to input() was used to pause the execution of the agent while testing the new signal handlers. It is no longer needed.
…5s after CTRL_CLOSE_EVENT signal The comment will warn us that in case that particular signal is raised, the cleanup shouldn't take longer than 5s
91bb2ec to
3f7c4a8
Compare
Agent will now handle interrupt and break signals on linux and windows
What does this PR do?
Fixes #1594
Handles SIGINT and SIGTERM for Linux
Handles SIGBREAK and window close event for Windows
PR Checklist
Is the TravisCI build passing?Was the CHANGELOG.md updated to reflect the changes?Was the documentation framework updated to reflect the changes?Testing Checklist
Added relevant unit tests?