kaxil commented on PR #73142: URL: https://github.com/apache/airflow/pull/73142#issuecomment-5841184872
Re-checked the rebase at 617d920. The `is not None` rule is correct against current main and replaces the `(SERVER_TERMINATED, UP_FOR_RETRY)` tuple from #73253 without losing anything. It also covers DEFERRED, UP_FOR_RESCHEDULE and AWAITING_INPUT, which reach `final_state` the same way SUCCESS does: the direct call succeeded and the pending slot was cleared. Two leftovers from before the rebase, neither blocking: 1. `test_confirmed_skipped_state_persists_over_later_nonzero_exit_code` now builds a state the supervisor can't reach. `_handle_task_state` always sets `_terminal_state` and `_pending_terminal_state_msg` together, so SKIPPED with nothing pending only exists in this test. With the message pending, main already calls `finish(state=skipped)`, so the docstring's "on pre-fix code ... writes FAILED over what should have been SKIPPED" no longer holds. What this change still fixes for `TaskState` is the reported state: after `wait()` dispatches `TaskState(FAILED)`, both coordinators read `final_state` into `ExecutionResult` and the "Workload finished" log, and with `should_retry=True` plus a non-zero exit, main reports `up_for_retry` for a row it just wrote as `failed`. Driving `_handle_task_state(TaskState(FAILED))`, then `update_task_state_if_needed()`, and asserting `final_state == FAILED` would pin that. Adding DEFERRED to the SUCCESS test's parametrize would cover the other direct-se nd states. 2. The docstring says the precedence "only matters if that property is ever consulted with `_terminal_state` set and no message pending", but that's the normal path for both coordinators (`coordinator.py:197`, `_subprocess.py:431`). At 29 lines it also reads more like the PR's history than a description of the property. The rule plus the SUCCESS example would be enough. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
