naruto-lgtm commented on code in PR #68662:
URL: https://github.com/apache/airflow/pull/68662#discussion_r3652442842
##########
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:
It's the fall-through policy. Any value that matches no branch ends up in
`default_serialization`, which with the default `strict=False` logs a debug
line and casts to `str(var)`, and with `strict=True` raises
`SerializationError("Encountered unexpected type")`. `test_strict_mode` is the
illustration, it serializes `[[[Test()]]]` both ways.
The reason it has to be forwarded on every recursive `cls.serialize(...)` is
that the unsupported object is usually nested inside a container, so if one
branch drops the kwarg then the caller asks for strict and still silently gets
a string back. That's the invariant
`test_recursive_serialize_calls_must_forward_kwargs` exists to protect.
It shows up in this diff because that guard test wasn't actually covering
the whole method. It AST-walks `serialize` and `break`s on the first
`.serialize` call whose receiver isn't `cls`, which was the `var.serialize()`
in the exception branch I removed. Everything later in walk order was never
checked, and the `LazySelectSequence` branch was calling
`cls.serialize(list(var))` with no `strict`, so a `LazySelectSequence` holding
an unserializable element would get stringified even under `strict=True`.
Switching the `break` to `continue` makes the walk cover the method, and the
forward at that call site is what it then asks for.
Latent rather than user-visible though: nothing outside the tests passes
`strict=True` today.
--
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]
