-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add support for SignalTask. #64
Conversation
Signed-off-by: Shishir Mahajan <[email protected]>
This patch can be tested using the following example job.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution. I want to discuss about the SIGINT fallback, otherwise it looks good to me.
7474374
to
b69009e
Compare
|
@towe75 I am contemplating on the error message. Which one do you think would be better?
OR
|
@shishir-a412ed I would go for the second one. Less words are usually better ;-) |
@towe75 Sounds good! I was leaning towards (2) as well. Let me update the PR. |
@towe75 Updated the error message. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Signed-off-by: Shishir Mahajan [email protected]