potiuk commented on code in PR #66574:
URL: https://github.com/apache/airflow/pull/66574#discussion_r3264689719
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -1254,6 +1277,50 @@ def update_task_state_if_needed(self):
rendered_map_index=self._rendered_map_index,
)
+ def _replay_pending_terminal_state_msg(self) -> None:
+ """
+ Re-issue the dedicated API call for an unsynced terminal-state msg.
+
+ Best-effort — if the second attempt also fails the exception is
+ logged and we move on; the supervisor's overall failure handling
+ (heartbeat, exit-code reporting) will eventually surface the issue.
+ """
+ msg = self._pending_terminal_state_msg
+ if msg is None:
+ return
+ try:
+ if isinstance(msg, SucceedTask):
+ self.client.task_instances.succeed(
+ id=self.id,
+ when=msg.end_date,
+ task_outlets=msg.task_outlets,
+ outlet_events=msg.outlet_events,
+ rendered_map_index=self._rendered_map_index,
+ )
Review Comment:
Good catch — extracted `_send_terminal_state_msg` so the four-branch
isinstance dispatch (succeed / retry / defer / reschedule) lives in one
place, and both `_handle_request` and `_replay_pending_terminal_state_msg`
go through it. The helper also owns the
`set pending → call API → clear pending → record state` sequence, so the
two call sites stay symmetric. See 156ba71.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
##########
task-sdk/tests/task_sdk/execution_time/test_supervisor.py:
##########
@@ -3023,6 +3023,75 @@ def test_handle_requests_api_server_error(self,
watched_subprocess, mocker):
"detail": error.response.json(),
}
+ def test_terminal_state_not_set_when_succeed_api_fails(self,
watched_subprocess, mocker):
Review Comment:
Done — both recovery tests are now parametrized over all four
terminal-state message types (`SucceedTask`, `RetryTask`, `DeferTask`,
`RescheduleTask`), so we cover `UP_FOR_RETRY` and `UP_FOR_RESCHEDULE`
in addition to the original `SUCCESS` / `DEFERRED` paths. See 156ba71.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]