guan404ming commented on code in PR #72155:
URL: https://github.com/apache/airflow/pull/72155#discussion_r3999672628
##########
providers/common/ai/src/airflow/providers/common/ai/operators/llm.py:
##########
@@ -135,8 +141,19 @@ 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 not (require_approval and
approval_timeout):
Review Comment:
Fixed: now requires `approval_timeout > timedelta(0)`, negative case tested,
docs bullets say positive.
##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -160,7 +170,7 @@ def defer_for_approval(
options=[LLMApprovalMixin.APPROVE, LLMApprovalMixin.REJECT],
subject=subject,
body=body,
- defaults=None,
+ defaults=timeout_defaults,
Review Comment:
Agreed it lives in core upsert; will address the existing-row branch in a
follow-up PR.
##########
providers/common/ai/docs/operators/llm.rst:
##########
@@ -223,6 +230,9 @@ Parameters
for human review. Default ``False``.
- ``approval_timeout``: Maximum time to wait for a review (``timedelta``).
``None``
means wait indefinitely. Default ``None``.
+- ``on_approval_timeout``: Outcome when ``approval_timeout`` expires without a
+ review: ``"fail"`` (default), ``"approve"``, or ``"reject"``. Requires
+ ``require_approval=True`` and ``approval_timeout``.
Review Comment:
Added the pre-highlight note to the docs page and both docstrings.
##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -62,16 +63,23 @@ class LLMApprovalMixin:
before approving. The (possibly modified) output is then returned as the
task result.
+ ``on_approval_timeout`` decides what happens when ``approval_timeout``
+ expires without a response: ``"fail"`` raises ``HITLTimeoutError``, while
+ ``"approve"`` and ``"reject"`` answer the review with that option so the
+ task resumes as if a reviewer had chosen it.
+
Operators that use this mixin must set the following attributes:
- ``require_approval`` (``bool``)
- ``allow_modifications`` (``bool``)
- ``approval_timeout`` (``timedelta | None``)
+ - ``on_approval_timeout`` (``str``)
Review Comment:
Updated the bullet to the Literal type.
--
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]
