You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "antonio-mello-ai (via GitHub)" <gi...@apache.org> on 2026/03/12 20:10:21 UTC
[PR] fix(providers/standard): add response_timeout to HITLOperator to prevent race with execution_timeout [airflow]
antonio-mello-ai opened a new pull request, #63475:
URL: https://github.com/apache/airflow/pull/63475
## Summary
- Add a dedicated `response_timeout` parameter to `HITLOperator` that controls how long the `HITLTrigger` waits for a human response
- This separates the human-response wait window from `BaseOperator.execution_timeout`, which wraps the entire `execute()` method and can kill the task before `defer()` is reached
- For backward compatibility, if `execution_timeout` is set without `response_timeout`, it is automatically migrated with a deprecation warning and `execution_timeout` is cleared
## Root Cause
`HITLOperator` reused `BaseOperator.execution_timeout` for two conflicting purposes:
1. **BaseOperator behavior**: wraps `execute()` in a timeout that kills the task
2. **HITL trigger timeout**: passed to `HITLTrigger` as the human response wait window
When `execution_timeout` is short (or notifiers are slow), the BaseOperator timeout fires **before** `self.defer()` is reached, killing the task with `AirflowTaskTimeout` instead of deferring.
## Test Plan
- [x] New test: `response_timeout` correctly passed to trigger
- [x] New test: `execution_timeout` backward compatibility emits deprecation warning and migrates
- [x] New test: both timeouts work together when `execution_timeout > response_timeout`
- [x] Updated existing timeout tests to use `response_timeout`
- [x] All 78 existing tests pass (1 skipped)
- [x] All pre-commit hooks pass (`prek`)
Closes #55866
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/standard): add response_timeout to HITLOperator to prevent race with execution_timeout [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #63475:
URL: https://github.com/apache/airflow/pull/63475#issuecomment-4059543137
I'm not convinced this is required. From worker to triggerer should'nt take that long and a short execution_time usually does not make sense
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/standard): add response_timeout to HITLOperator to prevent race with execution_timeout [airflow]
Posted by "antonio-mello-ai (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/standard): add response_timeout to HITLOperator to prevent race with execution_timeout [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #63475:
URL: https://github.com/apache/airflow/pull/63475#issuecomment-4073509133
The task is basically waiting for a response, so I think it's okay to use `execution_timeout`. The notifier part is somewhat rare, but could be. thoughts ?@jason810496 @guan404ming
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/standard): add response_timeout to HITLOperator to prevent race with execution_timeout [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W merged PR #63475:
URL: https://github.com/apache/airflow/pull/63475
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org