hwang-cadent commented on code in PR #56973:
URL: https://github.com/apache/airflow/pull/56973#discussion_r2809405989
##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -1587,6 +1587,11 @@ 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"):
+ # Check if value is ARG_NOT_SET before trying to deserialize as
datetime
+ if isinstance(value, dict) and value.get(Encoding.TYPE) ==
DAT.ARG_NOT_SET:
+ from airflow.serialization.definitions.notset import NOTSET
+
+ return NOTSET
Review Comment:
> Should this not return None instead? Not sure though?
Returning NOTSET is intentional and consistent with the general
deserialization pattern:
1. Consistency: The general deserialize() method (line 697-700) returns
NOTSET when type_ == DAT.ARG_NOT_SET, so i follow the same pattern for date
fields.
2. Semantic distinction: TriggerDagRunOperator distinguishes:
NOTSET → "parameter not provided, use default" → uses utcnow() (line 225-227)
None → "explicitly set to None" → uses None (line 228-229)
3. Operator behavior: The operator's __init__ (line 212-213) checks for
NOTSET and preserves it, and execute() handles NOTSET by using utcnow().
If you prefer None for simplicity, I can change it. Returning None would
also work since the operator handles None correctly (it falls back to utcnow()
when needed). Should I change it to return None instead?
--
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]
