-
Notifications
You must be signed in to change notification settings - Fork 44
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 halting execution plans #406
Conversation
This sounds like a nice feature, but I do not feel comfortable enough to ellaborate on the consequences here... |
Up until now, Dynflow only had support for clean cancellation. Upon cancellation a cancel event was sent to all cancellable suspended and running steps and it was the action's responsibility to cancel itself. However, there were actions which just weren't cancellable at all. This commit adds support for halting execution plans. The actions are completely unaware of this, on halt dynflow just destroys its internal state about a given execution plan and turns it to stopped state. It does not remove running steps from workers as we currently don't have any real means of doing so. Any pending events will be rejected. In other words, halting an execution plan ensure it will not run any further.
It is the caller's responsibility to not try to halt an already stopped execution plan.
@adamruzicka if you'd like me to give it another try, I can do a review here. I've improved my code reading skills and I've learned YOLO, so I'd be happy to merge after a proper review |
I was about to merge this myself in the upcoming weeks, but I'll never say no to another pair of eyes on a pr so if you could, I'd be grateful for that |
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.
Overall looks pretty well designed :)
There is one too many halted
flags to my taste and I'm unsure if the clean-up is all correct, but looks like it! :)
+1 from me FWIW.
Thank you @ezr-ondrej ! |
Up until now, Dynflow only had support for clean cancellation. Upon cancellation
a cancel event was sent to all cancellable suspended and running steps and it
was the action's responsibility to cancel itself. However, there were actions
which just weren't cancellable at all.
This commit adds support for halting execution plans. The actions are completely
unaware of this, on halt dynflow just destroys its internal state about a given
execution plan and turns it to stopped state. It does not remove running steps
from workers as we currently don't have any real means of doing so. Any pending
events will be rejected. In other words, halting an execution plan ensure it
will not run any further.