mage: cancel context on SIGINT#313
Conversation
On receiving an interrupt signal, mage cancels the context allowing the magefile to perform any cleanup before exiting. A second interrupt signal will kill the magefile process without delay. The behaviour for a timeout remains unchanged (context is canclled and the magefile exits).
b4d47e8 to
0aa93f8
Compare
|
Updated to switch test from |
|
You may want to consider holding off on this until |
|
Sorry for the late review. re: signal.NotifyContext .... I don't want mage to require a new version of Go, because I know the pain of being stuck on an older version. I think we can do fine with the knobs we have. This PR looks pretty good, but I think we're missing a synchronization effort... we should wait for the targets to clean up, and right now there's no mechanism for that. So, like, if you put a defer in a target that takes a little time to run, it may not always have time to run before mage exits out. I think we need a waitgroup and a timeout... like, first time we get sigint, cancel the context and start waiting for people to clean up. Maybe wait up to 5 seconds. If the user hits ctrl-c again, skip the timeout and just exit. so something like this: |
|
@natefinch thanks for the review. I've updated the code to apply a timeout after a SIGINT and tests to cover this, aswell as testing that a deferred function call within a target will be ran during during cleanup. I'm not sure where a WaitGroup would be useful? |
|
Hi can we get some movement on this PR @natefinch? This feature would be killer for us. Please let me know how I can help make it happen. Thank you |
|
Thanks for pinging me on this, @faiq ... I'll have to re-review the code and work on the merge conflicts, but it seems like it should be able to be merged soon. I'll try to poke at this some tonight. |
…alls to different targets
On receiving an interrupt signal, mage cancels the context allowing the magefile
to perform any cleanup before exiting.
A second interrupt signal will kill the magefile process without delay.
The behaviour for a timeout remains unchanged (context is canclled and the magefile
exits).
Fixes #269, #266.