guan404ming commented on code in PR #72155:
URL: https://github.com/apache/airflow/pull/72155#discussion_r3965654227
##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -179,7 +189,7 @@ def defer_for_approval(
trigger=HITLTrigger(
ti_id=ti_id,
options=[LLMApprovalMixin.APPROVE, LLMApprovalMixin.REJECT],
- defaults=None,
+ defaults=timeout_defaults,
Review Comment:
Good catch, dropped the defer timeout so the trigger deadline is the single
authority.
##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -219,6 +230,8 @@ def execute_complete(self, context: Context,
generated_output: str, event: dict[
responded_by_user = event.get("responded_by_user")
chosen = event["chosen_options"]
if self.APPROVE not in chosen:
+ if event.get("timedout"):
+ raise HITLRejectException("Output was rejected by the approval
timeout default.")
Review Comment:
Thanks, params_input is now ignored on timeout so stale attempt params never
auto-approve.
##########
providers/common/ai/src/airflow/providers/common/ai/operators/llm.py:
##########
@@ -135,8 +141,18 @@ def __init__(
self._serialize_model_output = serialize_output or not _CORE_WALKER
self.agent_params = agent_params or {}
self.usage_limits = usage_limits
+ if on_approval_timeout not in ("fail",
*LLMApprovalMixin.TIMEOUT_DEFAULTS):
+ raise ValueError(
+ f"on_approval_timeout must be 'fail', 'approve', or 'reject',
got {on_approval_timeout!r}."
+ )
+ if on_approval_timeout != "fail" and approval_timeout is None:
Review Comment:
Fixed, the guard now also requires require_approval=True; docs updated to
match.
--
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]
