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

Add an option to cancel a paused flow run instead of resuming it #7807

Closed
3 tasks done
anna-geller opened this issue Dec 7, 2022 · 4 comments · Fixed by PrefectHQ/prefect-ui-library#1127
Closed
3 tasks done
Assignees
Labels
enhancement An improvement of an existing feature status:accepted We may work on this; we will accept work from external contributors status:in-progress Someone is working on this, check-in here before beginning new work

Comments

@anna-geller
Copy link
Contributor

First check

  • I added a descriptive title to this issue.
  • I used the GitHub search to find a similar request and didn't find it.
  • I searched the Prefect documentation for this feature.

Prefect Version

2.x

Describe the current behavior

A common use case for the pause/resume functionality introduced in Prefect 2.7 is the ability to add a manual step to a workflow, such as checking data quality or validating the output of the current computation (manual step performed by a human).

Currently, Prefect only allows the Paused flow run to move into a Running state.

Describe the proposed behavior

It would be great if a Paused flow run could transition into a Cancelled state with a utility cancel_flow_run and potentially add a new Cancel button in the UI:

image

Example Use

manual quality check didn't pass, and the flow run shouldn't be resumed (should be canceled)

there's a need to be able to do that from the UI for business users performing such manual validation

Additional context

requested during a workshop at LiveEO

@anna-geller anna-geller added enhancement An improvement of an existing feature v2 labels Dec 7, 2022
@zanieb
Copy link
Contributor

zanieb commented Dec 7, 2022

PAUSED -> CANCELLED seems reasonable to me!

I think we should be able to attach data in the Stata.data field on resume which could facilitate an "approve or deny" workflow as well where the flow does resume but can take action based on the response.

@zanieb zanieb added status:accepted We may work on this; we will accept work from external contributors priority:medium and removed status:triage labels Dec 8, 2022
@cicdw cicdw self-assigned this Feb 7, 2023
@github-actions github-actions bot added the status:in-progress Someone is working on this, check-in here before beginning new work label Feb 7, 2023
@jbnitorum
Copy link

What are the expected results on this? My experience is that when you hit this cancel button on an in-process pause it moves to cancelling then cancelled. For an out of process pause it moves to cancelling and then stays there indefinitely. I guess this makes sense from the standpoint that if nothing is running underneath it can't move states (from cancelling to cancelled), but just from a user experience side of things it would be a little cleaner if the out of process pauses ended with cancelled as well.

Separately, does it present any problems if our cancellation workflow when cancelling flows from the API is to move paused flows (for us will always be out of process) directly to Cancelled (rather than Cancelling) using the set_state endpoint? It looks like this is the same endpoint the cancel button on the UI is using.

@jbnitorum
Copy link

Alternatively if changing flow state could be added as an automation action you could clean this up with:

If flow run stays in state cancelling for 30 minutes change flow state to cancelled.

Currently there is an automation action to cancel a flow run but that just puts it in state Cancelling as well. I guess we could get there with running a deployment that moves the state but less steps would be preferable.

@EmilRex
Copy link
Contributor

EmilRex commented Mar 22, 2023

@jbnitorum My understanding is that an agent should attempt to advance an out-of-process cancelling state to cancelled. You can see the expected behavior here. This does seem to work, at least for the Process infrastructure. After cancelling a reschedulable flow in Cloud I see this in the agent logs:

13:36:48.449 | INFO    | prefect.agent - Found 1 flow runs awaiting cancellation.
13:36:48.836 | INFO    | prefect.agent - Killing process Prefect-Mac.local:34959 for flow run '91ef2b5e-da7a-4d15-92bc-08dd7e925f72'...
13:36:48.837 | WARNING | prefect.agent - Unable to kill process 34959: The process was not found. Marking flow run as cancelled.

The latest relevant release I see is 2.7.11, so if you're running that or newer and still having issues, then it sounds like there may be a bug.

Separately, does it present any problems if our cancellation workflow when cancelling flows from the API is to move paused flows (for us will always be out of process) directly to Cancelled (rather than Cancelling) using the set_state endpoint?

I don't believe so, assuming the necessary infrastructure cleanup happens, which it does in your out-of-process case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature status:accepted We may work on this; we will accept work from external contributors status:in-progress Someone is working on this, check-in here before beginning new work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants