seanmuth commented on PR #69058: URL: https://github.com/apache/airflow/pull/69058#issuecomment-4845748959
Pushed `2352e1f453` addressing the review feedback: - **Short-circuit when requeues are disabled** — the task-instance state DB lookup is now skipped entirely when `pod_launch_failure_retries = 0`. - **Consolidated per-key state** — the job spec, requeue count, and the pod each requeue was last issued for now live in a single `_PodLaunchAttempt` per key, cleaned in one place across all exit paths. This also fixes the requeue count leaking when a pod is adopted (the previous separate counter was never cleared on the ADOPTED path). - **Dedupe duplicate `Failed` events** — Kubernetes can emit several `Failed` events for one pod (e.g. MODIFIED×2 + DELETED on SIGTERM); each previously triggered another requeue. The executor now records the pod a requeue was issued for and ignores duplicate events for that same pod, while a distinct (requeued) pod still requeues. New test: `test_change_state_pre_execution_failure_dedupes_repeated_events`. - **Removed dead `RUNNING` branch** — the watcher never emits `RUNNING`, so the cleanup there was dead code. - **Docs / changelog** — documented the default-behavior change (pre-execution pod failures are now requeued once before failing; set `pod_launch_failure_retries = 0` to restore the previous immediate-fail behavior) and the warning that `-1` (unlimited) with a crash-on-launch image accumulates pods under the default `delete_worker_pods_on_failure = False`. Full `TestKubernetesExecutor` suite passes locally (124 passed, 1 skipped). Per-thread replies inline. --- Drafted-by: Claude Code (Opus 4.8); reviewed by @seanmuth 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]
