Re: [PR] Skip notifier templating with empty template_fields [airflow]

["jroachgolf84 (via GitHub)" <gi...@apache.org>]

jroachgolf84 commented on code in PR #64993:
URL: https://github.com/apache/airflow/pull/64993#discussion_r3331405454


##########
task-sdk/tests/task_sdk/bases/test_notifier.py:
##########
@@ -97,3 +105,28 @@ def test_notifier_call_with_prepared_context(self, caplog):
             }
         )
         assert notifier.message == "task: some_task"
+
+    def test_notifier_call_with_serialized_dag_and_no_template_fields(self):
+        with DAG("test_notifier_call_with_serialized_dag_and_no_template_fields") as dag:
+            EmptyOperator(task_id="test_id")
+
+        notifier = NoTemplateNotifier()
+        notifier.notify = MagicMock()
+        serialized_dag = SerializedDAG.deserialize_dag(SerializedDAG.serialize_dag(dag))
+        context: Context = {"dag": serialized_dag}
+
+        notifier(context)
+
+        notifier.notify.assert_called_once_with({"dag": serialized_dag})
+
+    def test_notifier_render_template_fields_with_serialized_dag(self):
+        with DAG("test_notifier_render_template_fields_with_serialized_dag") as dag:
+            EmptyOperator(task_id="test_id")
+
+        notifier = MockNotifier(message="Hello {{ dag.dag_id }}")

Review Comment:
   Why are you using the `MockNotifier` here, but not in the test above?



##########
task-sdk/tests/task_sdk/bases/test_notifier.py:
##########
@@ -97,3 +105,28 @@ def test_notifier_call_with_prepared_context(self, caplog):
             }
         )
         assert notifier.message == "task: some_task"
+
+    def test_notifier_call_with_serialized_dag_and_no_template_fields(self):
+        with DAG("test_notifier_call_with_serialized_dag_and_no_template_fields") as dag:
+            EmptyOperator(task_id="test_id")
+
+        notifier = NoTemplateNotifier()
+        notifier.notify = MagicMock()
+        serialized_dag = SerializedDAG.deserialize_dag(SerializedDAG.serialize_dag(dag))

Review Comment:
   Can you explain the logic here? I'm not sure that I get why you're serializing the DAG, then deserializing it, all while using the name `serialized_dag`.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org