antonio-mello-ai commented on PR #63475: URL: https://github.com/apache/airflow/pull/63475#issuecomment-4060026367
Thanks for the review, @Lee-W. You're right that worker-to-triggerer is normally fast, and short `execution_timeout` values are uncommon. But the core issue here isn't about speed — it's a **semantic conflict**: `execution_timeout` on `BaseOperator` wraps the entire `execute()` method and kills the task on expiry. HITLOperator reuses that same parameter to compute the trigger's response deadline. These are two fundamentally different concerns: 1. **Task execution guard** (how long `execute()` can run before being killed) 2. **Human response window** (how long the trigger waits for input after deferral) When notifiers are involved (e.g., `SmtpNotifier`), the time before `defer()` is not negligible. With `execution_timeout=timedelta(seconds=30)` and a slow SMTP server, the task can be killed by BaseOperator's timeout *before* it even defers — which defeats the entire purpose of HITL. Even without notifiers, the current design means users can't independently control "how long my task is allowed to run" vs "how long to wait for human input." These are separate knobs that shouldn't be coupled. The fix is backward-compatible (auto-migrates `execution_timeout` → `response_timeout` with deprecation warning), so existing users aren't affected. Happy to discuss alternative approaches if you see a simpler path, but I believe the semantic separation is the right fix here. -- 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]
