Skip to content

Cancel running workflow - #14225

Open
erlenlh wants to merge 6 commits into
store-workflow-outputfrom
cancel-running-workflow
Open

Cancel running workflow#14225
erlenlh wants to merge 6 commits into
store-workflow-outputfrom
cancel-running-workflow

Conversation

@erlenlh

@erlenlh erlenlh commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14090

Approach
Update jobs to follow proper cancelation logic.
Stop cancel() from blocking UI/CLI thread.
Minor UI cleanup in workflow dialog.

(Screenshot of new behavior in GUI if applicable)
bilde

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Makes workflow cancellation non-blocking and adds cooperative cancellation support for internal jobs and exports.

Changes:

  • Removes blocking waits from WorkflowRunner.cancel().
  • Adds cancellation polling and regression tests.
  • Updates workflow UI layout, labels, and documentation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/ert/workflow_runner.py Makes cancellation non-blocking and documents behavior.
src/ert/config/ert_script.py Documents cooperative cancellation.
src/ert/plugins/hook_implementations/workflows/csv_export.py Stops processing ensembles after cancellation.
src/ert/plugins/hook_implementations/workflows/export_rft.py Stops exporting realizations after cancellation.
src/ert/gui/ertwidgets/closabledialog.py Supports left-aligned dialog buttons.
src/ert/gui/tools/workflows/workflows_tool.py Moves the start button into the dialog footer.
src/ert/gui/tools/workflows/workflow_dialog.py Renames the running-dialog action to Cancel.
src/ert/gui/tools/workflows/run_workflow_widget.py Cleans up workflow controls and cancellation UI.
tests/ert/unit_tests/workflow_runner/workflow_common.py Adds an uncooperative internal-job fixture.
tests/ert/unit_tests/workflow_runner/test_workflow_runner.py Tests non-blocking cancellation.
tests/ert/unit_tests/resources/test_csv_export.py Tests CSV export cancellation.
tests/ert/unit_tests/plugins/test_export_rft.py Tests RFT export cancellation.
docs/ert/reference/workflows/workflows.rst Documents cooperative cancellation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ert/workflow_runner.py Outdated
Comment thread src/ert/workflow_runner.py Outdated
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#88

@codecov-commenter

codecov-commenter commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.81%. Comparing base (dedec36) to head (f80fa2e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                    Coverage Diff                    @@
##           store-workflow-output   #14225      +/-   ##
=========================================================
+ Coverage                  91.79%   91.81%   +0.02%     
=========================================================
  Files                        486      486              
  Lines                      34902    34908       +6     
=========================================================
+ Hits                       32038    32051      +13     
+ Misses                      2864     2857       -7     
Flag Coverage Δ
cli-tests 36.00% <41.02%> (+0.01%) ⬆️
fuzz 43.88% <5.12%> (-0.01%) ⬇️
gui-tests 58.29% <53.84%> (-0.01%) ⬇️
performance-and-unit-tests 81.11% <89.74%> (+0.03%) ⬆️
test 45.50% <35.89%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/gui/ertwidgets/closabledialog.py 100.00% <100.00%> (ø)
src/ert/gui/tools/workflows/run_workflow_widget.py 75.22% <100.00%> (-0.19%) ⬇️
src/ert/gui/tools/workflows/workflow_dialog.py 87.50% <100.00%> (ø)
src/ert/gui/tools/workflows/workflows_tool.py 100.00% <100.00%> (ø)
...ugins/hook_implementations/workflows/csv_export.py 75.34% <100.00%> (+0.69%) ⬆️
...ugins/hook_implementations/workflows/export_rft.py 100.00% <100.00%> (ø)
src/ert/workflow_runner.py 95.18% <100.00%> (+0.27%) ⬆️

... and 1 file with indirect coverage changes

@codspeed-hq

codspeed-hq Bot commented Aug 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing cancel-running-workflow (774c756) with store-workflow-output (4b39c2a)

Open in CodSpeed

ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 20, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 20, 2026
@xjules

xjules commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

You need to rebase

Comment thread src/ert/config/ert_script.py Outdated
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 774c756 to 12fdc57 Compare August 27, 2026 07:09
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Aug 27, 2026
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 12fdc57 to 9282df1 Compare August 27, 2026 18:40
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshot tests now pass. The baseline update PR equinor/ert-testdata#88 has been closed.

@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 9282df1 to 4486d36 Compare August 28, 2026 08:11
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 4486d36 to 36b5b1e Compare August 31, 2026 11:00
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 36b5b1e to 4372356 Compare August 31, 2026 12:49
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 4372356 to 7186524 Compare September 1, 2026 12:09
erlenlh and others added 4 commits September 3, 2026 09:37
Cancelling an internal ErtScript workflow job is cooperative: cancel()
only sets a flag, nothing forcibly interrupts a running run(). A job
that never checks isCancelled() runs to completion regardless of the
user's request, and none of the jobs ERT ships did.

Document the contract on ErtScript.isCancelled() and in the workflow
reference, and adopt it in the two bundled jobs that loop over
realizations or ensembles: CSVExportJob and ExportRFTJob. Both now stop
before doing further work once cancellation is requested, without
losing what they already produced.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WorkflowRunner.cancel() used to call wait(), which blocks until the
currently running job actually stops. For an internal ErtScript that
does not poll isCancelled(), this can block for as long as the job
keeps running - and since cancel() is called directly on the Qt main
thread (GUI) or the CLI's main thread, this freezes the whole
application until the job finishes on its own.

cancel() is now purely a 'signal, don't block' call: it still asks the
current job to stop (which forcibly terminates external jobs), but no
longer waits for that to happen. Callers that need to block until the
workflow has actually stopped can still call wait() explicitly.

Add a regression test using an internal job that deliberately never
checks isCancelled(), proving cancel() returns promptly regardless.
Make 'Start workflow' a plain button matching 'Close' in size and
style (no icon, no oversized tool button), placed in the bottom-left
corner so it lines up with 'Close' in the bottom-right - instead of
sitting as an oversized row inside the form above.

Also rename the 'Running workflow' progress dialog's single button
from 'Close' to 'Cancel', since clicking it has only ever cancelled
the running workflow, never just closed the dialog.
The user already confirmed the cancellation via the 'Confirm cancel'
dialog, and the 'Running workflow' progress dialog simply closes.
A second 'was killed successfully!' popup afterwards is redundant.
erlenlh and others added 2 commits September 3, 2026 09:37
WorkflowRunner.cancel() could race with run_blocking() publishing the
next WorkflowJobRunner as __current_job, causing cancel() to target a
stale job. Even after publication, WorkflowJobRunner.cancel() was a
no-op until run() had assigned its ErtScript, so a job could start
without ever receiving the cancellation.

Add a lock around checking/publishing __current_job in WorkflowRunner
so cancel() always reads the job that is actually about to run. Add a
pending-cancel flag to WorkflowJobRunner, applied to the script as
soon as it is created, so a cancel() arriving before the script exists
is no longer lost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@erlenlh
erlenlh force-pushed the cancel-running-workflow branch from 7186524 to f80fa2e Compare September 3, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workflows should be cancelable

6 participants