kaxil opened a new pull request, #73261: URL: https://github.com/apache/airflow/pull/73261
Human-in-the-loop review needs Airflow 3.1+, but `require_approval=True` was never gated on it. The provider floors `apache-airflow>=3.0.0`, so a 3.0.x core is a supported install, and there the Dag imported cleanly and the failure landed mid-task. `execute` calls `agent.run_sync()` before `defer_for_approval`, and `defer_for_approval` imports `HITLTrigger`, which raises on import below 3.1, and `airflow.sdk.execution_time.hitl`, which does not exist before 3.1. So the model call was paid for, the task then died on an import that could never have succeeded, and every retry paid for it again. `__init__` now rejects the parameter the way `AgentOperator` already rejects `enable_hitl_review`, so the Dag fails to import instead. **Why `__init__` rather than `execute`.** Failing before the spend is the whole point, and a guard in `execute` would still sit downstream of `agent.run_sync()`, reporting the problem after the cost it exists to avoid. Placement within `__init__` matters too: the check sits below the `on_approval_timeout` literal validation and above the combination rule, so a typo keeps its own precise message, while on an old core the core version is reported rather than a combination error that would send the user to drop an argument that was never the problem. Verified against a real Airflow 3.0.6 core with the provider installed from source: | | before | after | |---|---|---| | Dag import | succeeds | fails with `AirflowOptionalProviderFeatureException: require_approval=True needs Airflow 3.1+.` | | `agent.run_sync()` | called, then the task fails | never reached |  The four `@task.llm*` decorator tests that build an operator with `require_approval=True` now carry a 3.1 skip, matching the existing gate on the equivalent `AgentOperator` test. Without it they error on the 3.0.6 row of the providers-compatibility job rather than passing. One caveat, noted in `llm.rst`: a dynamically mapped task (`.expand()`) is only constructed when it runs, so there the same error surfaces as a task failure rather than an import error, still before the model is called. Related to #72157, which adds this kind of guard for `approval_assigned_users`. This covers the parent parameter that knob depends on. -- 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]
