unsigned-long-int commented on code in PR #59530:
URL: https://github.com/apache/airflow/pull/59530#discussion_r2747452646
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -2002,7 +2002,10 @@ def _deserialize_field_value(cls, field_name: str,
value: Any) -> Any:
elif field_name == "resources":
return Resources.from_dict(value) if value is not None else None
elif field_name.endswith("_date"):
- return cls._deserialize_datetime(value) if value is not None else
None
+ # type security first: _deserialize_datetime expects numeric for
serialization
+ if isinstance(value, (int, float)) and not isinstance(value, bool):
Review Comment:
> hey @uranusjr
>
> okay let me explain the issue in more details.
>
> Under `airflow-core/src/airflow/serialization/serialized_objects.py`
`SerializedBaseOperator.serialize_mapped_operator`, when
`MyOp.partial(from_date="{{...}}", to_date="{{...}}")` these template fields
are stored in `partial kwargs` as string.
>
>
>
> ```python
>
> @classmethod
>
> def serialize_mapped_operator(cls, op: MappedOperator) -> dict[str,
Any]:
>
> # ...
>
> if op.partial_kwargs:
>
> serialized_op["partial_kwargs"] = {}
>
> for k, v in op.partial_kwargs.items():
>
> if cls._is_excluded(v, k, op):
>
> continue
>
>
>
> if k in [f"on_{x}_callback" for x in ("execute",
"failure", "success", "retry", "skipped")]: # skipped here for from_date
>
> if bool(v):
>
> serialized_op["partial_kwargs"][f"has_{k}"] = True
>
> continue
>
> serialized_op["partial_kwargs"].update({k:
cls.serialize(v)}) # here serialization is called
>
> #....
>
> return serialized_op
>
>
>
> ```
>
>
>
> `SerializedBaseOperator.serialize`
>
> Here we call serialize from `SerializedBaseOperator` which in turn
delegates to `serialize` classmethod from `BaseSerialization`
>
> ```python
>
> @classmethod
>
> def serialize(cls, var: Any, *, strict: bool = False) -> Any:
>
> # the wonders of multiple inheritance BaseOperator defines an
instance method
>
> return BaseSerialization.serialize(var=var, strict=strict)
>
> ```
>
>
>
> Here in `serialize` classmethod from `BaseSerialization`, we validate if
the field is primitive, which it is.
>
>
>
>
>
> ```python
>
> @classmethod
>
> def serialize(
>
> cls, var: Any, *, strict: bool = False
>
> ) -> Any:
>
>
>
> elif cls._is_primitive(var): # this is hit, since string is
primitive.
>
> if isinstance(var, enum.Enum):
>
> return var.value
>
> if isinstance(var, float) and (math.isnan(var) or
math.isinf(var)):
>
> return str(var)
>
> return var # default case is hit
>
>
>
>
>
> # is_primitive is hit, because field value is string
>
> _primitive_types = (int, bool, float, str)
>
> # ....
>
> @classmethod
>
> def _is_primitive(cls, var: Any) -> bool:
>
> """Primitive types."""
>
> return var is None or isinstance(var, cls._primitive_types)
>
> ```
>
>
Hey @uranusjr - please review the explanation and let me know if anything
here is unclear.
--
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]
