kaxil commented on code in PR #72155:
URL: https://github.com/apache/airflow/pull/72155#discussion_r3997968739
##########
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:
The Protocol 31 lines up is now `Literal["fail", "approve", "reject"]`, so
this bullet is the last place saying `str`. Worth matching, since this is the
contract a non-`LLMOperator` implementor reads, and `defer_for_approval` looks
the value up with `TIMEOUT_DEFAULTS.get(...)`, which returns `None` rather than
raising for anything outside the set.
##########
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:
Worth a sentence here that `on_approval_timeout` also changes what the
reviewer sees, since it writes the shared `HITLDetail.defaults` column.
`isHighlightOption`
(`airflow-core/src/airflow/ui/src/pages/HITLTaskInstances/HITLResponseForm.tsx:44-60`)
falls back to `defaults` while no response has arrived, and drives
`colorPalette` (`:139`) and `variant` (`:144`): with `defaults` null both
buttons render brand/solid, `["Approve"]` greys out Reject, and `["Reject"]`
greys out Approve. So `on_approval_timeout="reject"` makes Reject the primary
button on an approval gate.
`HITLOperator` documents its own `defaults` as both roles
(`standard/operators/hitl.py:65`); the wrapper here describes only the timeout
half, on all five pages and in the mixin docstring.
--
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]
