Lee-W commented on code in PR #74695:
URL: https://github.com/apache/airflow/pull/74695#discussion_r4521028446


##########
providers/common/ai/src/airflow/providers/common/ai/utils/output_type.py:
##########
@@ -38,6 +46,59 @@
 _CONVERSION_ERRORS = (ValidationError, ValueError, TypeError, AttributeError, 
KeyError, IndexError)
 
 
+def is_model_dump_required(output: Any, output_type: Any) -> bool:
+    """
+    Tell whether an output holding ``BaseModel`` instances must be pushed to 
XCom dumped.
+
+    The instances are looked up in the output itself, as 
``_iter_models_in_output`` walks it: the elements
+    of a list, tuple (a NamedTuple included), set or frozenset, and the keys 
and values of a dict. A downstream task can load a model from XCom only if the 
worker
+    registered the model's class, and the worker registers the classes 
``iter_pydantic_models`` finds in
+    ``output_type``. It finds none in a list of output types, an output 
marker, an output function, a
+    ``TypeAliasType`` or a ``NewType``, so an instance of a class it did not 
find is dumped. Without
+    ``iter_pydantic_models`` there is no evidence that a class is 
unregistered, so nothing is dumped.
+    """
+    if iter_pydantic_models is None:

Review Comment:
   Without `iter_pydantic_models`, `is_model_dump_required` now treats the 
registered set as empty, so a model inside a list, tuple or dict is dumped, as 
`AgentOperator` did in 1.1.0. The LLM operators go through the same helper, so 
their `list[M]` is a dict before Airflow 3.3 as well, and the changelog says so.



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