naruto-lgtm commented on code in PR #68662:
URL: https://github.com/apache/airflow/pull/68662#discussion_r3652428334
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -582,7 +561,7 @@ def serialize(
elif isinstance(var, XComArg):
return cls._encode(serialize_xcom_arg(var), type_=DAT.XCOM_REF)
elif isinstance(var, LazySelectSequence):
- return cls.serialize(list(var))
+ return cls.serialize(list(var), strict=strict)
Review Comment:
`strict` only controls the fallback at the bottom of `serialize`: a value
that matches no branch goes to `default_serialization`, which returns
`str(var)` when strict is False (the default) and raises
`SerializationError("Encountered unexpected type")` when it's True. So it's a
fail-loud switch for callers who would rather get an error than silently end up
with a stringified object. Nothing in tree passes `strict=True` outside
`test_strict_mode`, though the kwarg is exposed on
`BaseSerialization.serialize`.
The reason it turns up in this diff at all:
`test_recursive_serialize_calls_must_forward_kwargs` walks the AST of
`serialize` and asserts every `cls.serialize(...)` recursion forwards the
kwonly args. It used to `break` on the first non-`cls` serialize call, and that
was `var.serialize()` in the exception encode branch, the third call in walk
order. So it only ever validated 2 of the 6 recursions and never reached the
`LazySelectSequence` line, which had been dropping `strict` silently. Removing
the exception branch removed the thing it was breaking on, so the walk now
covers the whole method and flags it. I forwarded it and swapped the `break`
for `continue` so the invariant actually holds.
No behaviour change for any current caller since strict defaults to False.
With `strict=True`, a `LazySelectSequence` holding an unserializable element
now raises instead of quietly stringifying it, which is the point of strict.
--
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]
