developer-rpai commented on code in PR #73748:
URL: https://github.com/apache/airflow/pull/73748#discussion_r4118968954
##########
task-sdk/src/airflow/sdk/serde/serializers/datetime.py:
##########
@@ -52,9 +52,19 @@ def serialize(o: object) -> tuple[U, str, int, bool]:
if isinstance(o, datetime):
qn = qualname(o)
- tz = serialize_timezone(o.tzinfo) if o.tzinfo else None
+ if o.tzinfo is None:
+ # A naive datetime carries no timezone information, so anchor it to
+ # the configured default timezone (``core.default_timezone``)
rather
+ # than the OS local timezone of the serializing process. Otherwise
+ # the stored epoch silently depends on which machine writes the
value.
+ # The payload keeps ``tz`` empty so it still deserializes as naive.
+ ts = make_aware(o).timestamp()
+ tz = None
+ else:
+ ts = o.timestamp()
+ tz = serialize_timezone(o.tzinfo)
- return {TIMESTAMP: o.timestamp(), TIMEZONE: tz}, qn, __version__, True
+ return {TIMESTAMP: ts, TIMEZONE: tz}, qn, __version__, True
Review Comment:
Done -- bumped to `__version__ = 3` and gated the read: v3+ tz-less payloads
use the new default-timezone interpretation, v1/v2 keep the legacy OS-local
read, so in-flight data does not silently shift during rolling upgrades. Good
catch on the trigger-kwargs angle too -- those go through the same serde path,
which is exactly why the version gate matters.
##########
task-sdk/src/airflow/sdk/serde/serializers/datetime.py:
##########
@@ -93,6 +103,12 @@ def deserialize(cls: type, version: int, data: dict | str)
-> datetime.date | da
)
if cls is datetime.datetime and isinstance(data, dict):
+ if tz is None:
+ # No timezone was stored, so this was a naive datetime. Interpret
the
+ # epoch in the configured default timezone (mirroring
``serialize``)
+ # and return a naive datetime, so the round-trip compares equal to
+ # what was pushed regardless of the deserializing process's OS
timezone.
Review Comment:
Consolidated -- each comment now says one thing: serialize notes the
writer-independent anchoring, deserialize notes the version gating.
##########
task-sdk/src/airflow/sdk/serde/serializers/datetime.py:
##########
@@ -93,6 +103,12 @@ def deserialize(cls: type, version: int, data: dict | str)
-> datetime.date | da
)
if cls is datetime.datetime and isinstance(data, dict):
+ if tz is None:
+ # No timezone was stored, so this was a naive datetime. Interpret
the
+ # epoch in the configured default timezone (mirroring
``serialize``)
+ # and return a naive datetime, so the round-trip compares equal to
+ # what was pushed regardless of the deserializing process's OS
timezone.
+ return
make_naive(datetime.datetime.fromtimestamp(float(data[TIMESTAMP]),
tz=datetime.timezone.utc))
Review Comment:
Fixed -- restructured that branch so the longest line is well under 110.
`ruff format --check` and `ruff check` pass on both changed files.
--
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]
