feat: support timezone in SDK temporal partition mappers#67164
Conversation
ca0c304 to
2c4fbf3
Compare
The core temporal mappers accept a `timezone` kwarg but the SDK `_BaseTemporalMapper` did not, so Dag authors could not reach the feature via `from airflow.sdk import StartOfDayMapper`. Complete the SDK signature by adding the kwarg to the constructor and carrying `_timezone` through the `_Serializer` dispatch handler so the core class receives it on deserialize.
2c4fbf3 to
408f7a0
Compare
|
|
||
| def __init__( | ||
| self, | ||
| *, |
There was a problem hiding this comment.
Adding *, here makes the SDK constructor keyword-only. That matches the Core class shape, which is the right destination, but it's a breaking signature change against the already-released task-sdk/1.2.1, where _BaseTemporalMapper.__init__ accepts input_format positionally — so StartOfDayMapper("%Y-%m-%dT%H:%M:%S") is valid public API today and would break on upgrade. Worth calling out in the PR description (and a newsfragment) since this is the public entry point Dag authors import via from airflow.sdk import StartOfDayMapper.
| input_format: str = "%Y-%m-%dT%H:%M:%S", | ||
| output_format: str | None = None, | ||
| ) -> None: | ||
| self._timezone = timezone |
There was a problem hiding this comment.
The Core _BaseTemporalMapper runs timezone through parse_timezone(...) before assigning, but the SDK side just stores it verbatim. So sdk_instance._timezone can be str | Timezone | FixedTimezone, while core_instance._timezone is always a Timezone | FixedTimezone. The encode path tolerates both (encode_timezone handles all three types), but it leaves a subtle type asymmetry on a _timezone attribute that lives on both sibling classes under the same name. Either mirror parse_timezone here, or add a brief comment noting the SDK keeps the raw value and normalization happens in Core.deserialize, so future readers don't trip on it.
Why
The core
_BaseTemporalMapperaccepts atimezonekwarg, but the SDK class that Dag authors actually instantiate (from airflow.sdk import StartOfDayMapper) did not — and the encoder dispatch dropped_timezoneeven if it had.What
timezonekwarg to SDK_BaseTemporalMapper.__init__, matching the core signature (stored verbatim; core'sdeserializehandles parsing)._timezonethrough the_Serializerdispatch handler for all sixStartOf*Mapperclasses.StartOfHourMapperimport inencoders.pyso singledispatch registration matches the runtime type.Was generative AI tooling used to co-author this PR?
Generated-by: [Claude] following the guidelines
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.