Re: [PR] Task SDK: don't let exit code override a confirmed terminal state [airflow]

["seanmuth (via GitHub)" <gi...@apache.org>]

seanmuth commented on PR #73142:
URL: https://github.com/apache/airflow/pull/73142#issuecomment-5718430401

   Pushed a squashed rewrite addressing this round of review:
   
   - Retitled and reworded the PR body and the single remaining commit to drop the retracted "unobserved exit code defaults to 1" framing entirely, so it no longer survives into squash-merge history. The docstring now cites the two verified reachable cases instead: `SUCCESS` (row already written by `succeed()`, a later non-zero exit re-triggers a 409ing `.finish()`) and `SKIPPED` (no direct API call, row still `RUNNING`, so a later non-zero exit either overwrites it with `FAILED` or -- with retries -- abandons the write entirely).
   - Fixed the test ordering bug: `update_task_state_if_needed()` and `finish.assert_not_called()` now run before the `final_state` assertion, so `should_retry=False` actually pins the spurious `.finish()` call instead of pytest stopping at the first (already-informative) assertion.
   - Added `test_confirmed_skipped_state_persists_over_later_nonzero_exit_code`, parametrized over both `should_retry` values, asserting `finish()` is called with `state=SKIPPED` -- covering the half of the behavior change (states outside `STATES_SENT_DIRECTLY`) the existing tests never exercised.
   
   Full suite green (211 passed, 1 skipped/platform-only) plus mypy clean after each change.
   
   ---
   Drafted-by: Claude Sonnet 5 (no human review before posting)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org