SameerMesiah97 commented on code in PR #76859:
URL: https://github.com/apache/airflow/pull/76859#discussion_r4529826935


##########
airflow-core/src/airflow/serialization/definitions/dag.py:
##########
@@ -185,6 +190,21 @@ def timetable(self, value: Timetable) -> None:
         self._timetable = value
         self._encoded_timetable = None
 
+    def get_template_env(self, *, force_sandboxed: bool = False) -> 
jinja2.Environment:
+        """
+        Build a Jinja2 environment using the primitives stored at 
serialization time.
+
+        Callables from ``user_defined_macros`` and all 
``user_defined_filters`` cannot
+        survive JSON serialization, so they are absent here.  DAGs that need 
callable
+        macros in a trigger context must load the bundle and re-import the DAG 
file.

Review Comment:
   Please trim this docstring. Its too long.



##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -1966,6 +1975,39 @@ def get_dag_optional_fields_from_schema(cls) -> set[str]:
         optional_fields = operator_fields - required_fields
         return optional_fields
 
+    @classmethod
+    def _serialize_user_defined_macros(cls, dag_id: str, macros: dict) -> dict:
+        """
+        Return the JSON-safe entries from *macros*, dropping callables with a 
warning.
+
+        Only ``str``, ``int``, ``float``, ``bool``, ``None``, ``list``, and 
``dict``
+        values (recursively composed of the same types) survive serialization.
+        Any callable is skipped — the triggerer cannot reconstruct it from 
stored
+        JSON, so DAG authors who need callable macros in a trigger context 
must load
+        the bundle and re-import the DAG file instead.
+        """
+        import json as _json
+
+        safe: dict = {}
+        dropped: list[str] = []
+        for key, value in macros.items():
+            if callable(value):
+                dropped.append(key)
+                continue
+            try:
+                _json.dumps(value)
+                safe[key] = value
+            except (TypeError, ValueError):
+                dropped.append(key)
+        if dropped:
+            log.warning(

Review Comment:
   Could we log dropped macros at DEBUG? This runs during DAG serialization, 
and the warning can repeat on subsequent parses even for DAGs that never use 
`start_from_trigger`. The comment also says this is a one-time warning per DAG, 
but there’s no deduplication here.



##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -1966,6 +1975,39 @@ def get_dag_optional_fields_from_schema(cls) -> set[str]:
         optional_fields = operator_fields - required_fields
         return optional_fields
 
+    @classmethod
+    def _serialize_user_defined_macros(cls, dag_id: str, macros: dict) -> dict:
+        """
+        Return the JSON-safe entries from *macros*, dropping callables with a 
warning.
+
+        Only ``str``, ``int``, ``float``, ``bool``, ``None``, ``list``, and 
``dict``
+        values (recursively composed of the same types) survive serialization.
+        Any callable is skipped — the triggerer cannot reconstruct it from 
stored
+        JSON, so DAG authors who need callable macros in a trigger context 
must load
+        the bundle and re-import the DAG file instead.

Review Comment:
   Please trim this docstring. Its too long.



##########
airflow-core/tests/unit/serialization/test_dag_serialization.py:
##########
@@ -661,6 +661,75 @@ def 
test_dag_serialization_preserves_empty_access_roles(self):
             "__var": {},
         }
 
+    @pytest.mark.db_test
+    def test_user_defined_macros_primitives_are_serialized(self):
+        """JSON-safe macro values survive a round-trip through 
DagSerialization."""
+        dag = DAG(
+            "macro_roundtrip_dag",
+            schedule=None,
+            start_date=datetime(2024, 1, 1),
+            user_defined_macros={
+                "env": "production",
+                "max_retries": 3,
+                "ratio": 0.75,
+                "flag": True,
+                "nested": {"key": "val"},
+                "items": [1, 2, 3],
+            },
+        )
+        serialized = DagSerialization.to_dict(dag)
+        DagSerialization.validate_schema(serialized)
+
+        raw_macros = serialized["dag"]["user_defined_macros"]
+        assert raw_macros["env"] == "production"
+        assert raw_macros["max_retries"] == 3
+        assert raw_macros["ratio"] == 0.75
+        assert raw_macros["flag"] is True
+        assert raw_macros["nested"] == {"key": "val"}
+        assert raw_macros["items"] == [1, 2, 3]
+
+        # Round-trip: the deserialized SerializedDAG should expose them
+        # through get_template_env() so templates render correctly.
+        ser_dag = DagSerialization.from_dict(serialized)

Review Comment:
   Could we round-trip the DAG payload through JSON and render a template with 
the restored environment?



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