Skip to content

fix(odin): Update stop action status to succeeded instead of canceled - #571

Merged
vikramlc-cognite merged 1 commit into
masterfrom
EDG-781-fix-stop-action-status
Aug 27, 2026
Merged

fix(odin): Update stop action status to succeeded instead of canceled#571
vikramlc-cognite merged 1 commit into
masterfrom
EDG-781-fix-stop-action-status

Conversation

@vikramlc-cognite

Copy link
Copy Markdown
Contributor

Summary

Fixes stop-task actions incorrectly reporting Canceled instead of Succeeded when they successfully stop the target task.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing behavior)
  • Refactor (no functional change)
  • Documentation update
  • Chore / tooling / CI

What changed

  • cognite/extractorutils/unstable/core/base.py::_handle_stop_task_action: report ActionStatus.succeeded (was ActionStatus.canceled) when the target task's CancellationToken is found and cancelled.
  • Updated the two affected assertions in tests/test_unstable/test_action_dispatch.py (test_stop_task_action_cancels_child_token_and_reports_succeeded, test_stop_action_cancels_boot_launched_continuous_task) to expect succeeded.

Why it changed

ActionStatus.canceled is meant to mean "this action was itself canceled," not "this action successfully performed a cancellation." A stop action that does its job correctly was showing up as aborted in Odin, misleading operators.

What to focus on during review

  • Confirm no downstream consumer (Odin UI/alerting) treats a stop action's terminal status as a signal distinct from succeeded in a way that depended on the old canceled value.

Test evidence

  • python -m pytest tests/test_unstable/test_action_dispatch.py -q → 27 passed.

Risks and unknowns

  • None identified; single status value change, failed/token-not-found path unaffected.

Rollout and rollback

  • No flags/migrations. Plain code revert if needed.

Checklist

  • Self-reviewed the diff
  • Tests added or updated (or N/A with reason)
  • Docs updated (or N/A) — N/A, internal status semantics, not user-facing docs
  • No secrets, credentials, or PII committed
  • Breaking changes called out above and communicated to affected teams — N/A, not breaking

@vikramlc-cognite
vikramlc-cognite requested a review from a team as a code owner August 26, 2026 05:44
@vikramlc-cognite vikramlc-cognite self-assigned this Aug 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the stop task action handler to report a status of ActionStatus.succeeded instead of ActionStatus.canceled when stopping a task. The corresponding unit tests have been updated to assert this new behavior. There are no review comments, and I have no additional feedback to provide.

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.01%. Comparing base (31b010f) to head (3b4f8ba).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #571      +/-   ##
==========================================
+ Coverage   83.97%   84.01%   +0.04%     
==========================================
  Files          46       46              
  Lines        4686     4686              
==========================================
+ Hits         3935     3937       +2     
+ Misses        751      749       -2     
Files with missing lines Coverage Δ
cognite/extractorutils/unstable/core/base.py 85.50% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vikramlc-cognite vikramlc-cognite added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Aug 26, 2026
@andersfylling

Copy link
Copy Markdown

I don't quite understand this? If I cancelled a task and I would expect it to show up as cancelled?

@vikramlc-cognite

Copy link
Copy Markdown
Contributor Author

I don't quite understand this? If I cancelled a task and I would expect it to show up as cancelled?

In this system, a Task (e.g. "Task1") is the actual running job - but a Task itself doesn't carry a succeeded/failed/canceled status. Only Actions do. When you click "Stop" on Task1, that's not a status change on Task1 directly - it dispatches a brand-new, separate Action called "Stop Task1" (type stop_task).

Meanwhile, Task1 is actually being run by a different Action - the "Start Task1" action (type start_task), which has been sitting there this whole time, blocked executing Task1's code until it finishes or is interrupted.

So when you cancel Task1, there are two separate Action records involved, each with its own independent status:

"Start Task1" - the action that's actually running the task. When "Stop Task1" interrupts it, this action correctly reports canceled - because the thing it was doing (running Task1) really was cut short. This is what you are picturing when you say "I cancelled a task, I'd expect it to show cancelled" - and it's still exactly true after this PR. Nothing about this changes.

"Stop Task1" - a separate, short-lived action whose only job is to signal "Start Task1" to stop. This is the one this PR touches. Before the fix, whenever "Stop Task1" successfully delivered that signal, it also reported its own status as canceled - which reads as if the stop request itself was aborted or failed, when actually it did exactly what it was supposed to do. After the fix, "Stop Task1" reports succeeded (the stop request worked), while "Start Task1" - the action that was actually running your task - still reports canceled, unaffected.

So the confusion is: you're thinking of Task1's own outcome (still canceled, unchanged), but the status this PR changes belongs to a different action - the "Stop" request itself, not the thing it stopped.

@kornelione kornelione left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🦄

@kornelione kornelione added risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action and removed waiting-for-risk-review Waiting for a member of the risk review team to take an action labels Aug 27, 2026
@vikramlc-cognite
vikramlc-cognite merged commit bd9b3cd into master Aug 27, 2026
6 checks passed
@vikramlc-cognite
vikramlc-cognite deleted the EDG-781-fix-stop-action-status branch August 27, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants