paultmathew commented on PR #67229:
URL: https://github.com/apache/airflow/pull/67229#issuecomment-4539523106

   @jscheffl @SameerMesiah97 thank you both for the reviews.
   
   Pulled the `**kwargs` revert and any new surface on `KubernetesPodTrigger`. 
The deadline now rides inside the existing `trigger_kwargs` dict under the 
reserved key `_execution_deadline`. The leading-underscore convention follows 
the `_redefer_count` precedent in the same file.
   
   @SameerMesiah97 you were right — the previous 60s minimum clamp didn't 
account for `poll_interval`. With `execution_timeout=30s, poll_interval=60s`, 
the framework would have cancelled the trigger at T+60 before its next deadline 
check at T+65.
   
   The latest commit guarantees `defer.timeout` always covers `remaining + ≥ 2 
poll cycles`. New unit test pins it: 
`test_invoke_defer_method_pads_defer_timeout_for_slow_poll_interval` verifies 
`defer.timeout = 30 + 120 = 150s` for a `poll_interval=60` setup. I also did a 
smoke test against a live Kubernetes cluster (EKS) in our sandbox environment.
   
   ## Smoke test summary
   
   | Task | Configuration | Result | What it proved |
   |---|---|---|---|
   | t1 happy path | `execution_timeout=5m`, sleep 20s | ✓ success at ~26s, pod 
deleted | Deferred path works end-to-end on real K8s; deadline doesn't trip 
prematurely |
   | t2 timeout default poll | `execution_timeout=30s, poll_interval=2s`, sleep 
600s | ✓ failed at ~64s with `"Execution deadline reached for pod ... emitting 
timeout event"`, pod deleted | New code path fires; `_clean()` runs; trigger 
emits soft-timeout event |
   | t3 timeout slow poll | `execution_timeout=30s, poll_interval=30s`, sleep 
600s | ✓ failed at ~70s, timeout event at +38s, pod deleted | The reviewer's 
exact concern — slow poll doesn't break cleanup; new `poll_buffer = max(60, 
poll_interval * 2)` gives the trigger sufficient runway |
   | t4 no timeout | no `execution_timeout`, sleep 20s | ✓ success at ~26s, no 
deadline messages | Opt-out path preserved; tasks without `execution_timeout` 
continue to work as before |


-- 
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]

Reply via email to