guan404ming commented on code in PR #72159:
URL: https://github.com/apache/airflow/pull/72159#discussion_r3986182439


##########
providers/common/ai/tests/unit/common/ai/mixins/test_approval.py:
##########
@@ -174,6 +178,31 @@ def test_array_schema_passes_list_param_value(
         defer_kwargs = approval_op_with_modifications.defer.call_args[1]
         assert defer_kwargs["kwargs"]["generated_output"] == '["task_a"]'
 
+    @patch(HITL_TRIGGER_PATH, autospec=True)
+    @patch(UPSERT_HITL_PATH)
+    def test_notifiers_fire_once_the_review_is_open(self, mock_upsert, 
mock_trigger_cls, context):

Review Comment:
   Added `test_notifiers_fire_before_awaiting_input` in 
`TestAwaitInputForApproval`.



##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -165,6 +175,9 @@ def defer_for_approval(
             params=hitl_params,
         )
 
+        for notifier in self.approval_notifiers:

Review Comment:
   Agreed; notifier failures now log and continue, the review stays open. Test 
and docs updated.



##########
providers/common/ai/src/airflow/providers/common/ai/mixins/approval.py:
##########
@@ -165,6 +175,9 @@ def defer_for_approval(
             params=hitl_params,
         )
 
+        for notifier in self.approval_notifiers:
+            notifier(context)

Review Comment:
   Notifier now gets `subject` and `body` merged into its context; documented 
as `{{ subject }}` / `{{ body }}`.



##########
providers/common/ai/docs/operators/llm.rst:
##########
@@ -205,6 +205,13 @@ approving with ``allow_modifications=True``, and set a 
deadline with
     :start-after: [START howto_operator_llm_approval]
     :end-before: [END howto_operator_llm_approval]
 
+A pending review is only visible on the Required Actions page.  Pass

Review Comment:
   Reworded to "is not surfaced as a notification".



##########
providers/common/ai/src/airflow/providers/common/ai/operators/llm.py:
##########
@@ -88,6 +88,8 @@ class LLMOperator(BaseOperator, LLMApprovalMixin):
     :param allow_modifications: If ``True``, the reviewer can edit the output
         before approving.  The modified value is returned as the task result.
         Default ``False``.
+    :param approval_notifiers: Notifiers called once the review is open, so a

Review Comment:
   Added the "Only takes effect with `require_approval=True`" note.



-- 
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]

Reply via email to