Skip to content

Make temporal mapper timezone aware#62709

Open
Lee-W wants to merge 2 commits intoapache:mainfrom
astronomer:make-temporal-mapper-timezone-aware
Open

Make temporal mapper timezone aware#62709
Lee-W wants to merge 2 commits intoapache:mainfrom
astronomer:make-temporal-mapper-timezone-aware

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Mar 2, 2026

Why

Currently, CronPartitionedTimetable does not generate a timezone-aware partition key. Temporal partition mappers do not support timezone either. This might make the result misleading

What

Make CronParitionedTimetable generate a timezone-aware partition key by default and make partition mappers parse the time info and then output a timezone aware partition key


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: [Claude] following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@Lee-W Lee-W self-assigned this Mar 4, 2026
@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch 3 times, most recently from 5811b38 to fdc550b Compare March 4, 2026 12:30
@Lee-W Lee-W marked this pull request as ready for review March 4, 2026 12:44
@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch from fdc550b to 496d9d6 Compare March 5, 2026 03:42
@uranusjr
Copy link
Member

uranusjr commented Mar 5, 2026

Maybe the argument should be optional, and default to the configured timezone? That’s what the cron expression schedule does (although we didn’t implement it at the timetable level; the default timezone is inherited from the DAG object)

@Lee-W
Copy link
Member Author

Lee-W commented Mar 5, 2026

Maybe the argument should be optional, and default to the configured timezone? That’s what the cron expression schedule does (although we didn’t implement it at the timetable level; the default timezone is inherited from the DAG object)

Didn't know we have that, feel like I'll need to tweak CronParitionedTimetable a bit. Let me play with it and see how it works

@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch 2 times, most recently from a4c2db5 to 3f8d15b Compare March 9, 2026 08:58
@Lee-W Lee-W marked this pull request as draft March 9, 2026 09:02
@uranusjr
Copy link
Member

uranusjr commented Mar 9, 2026

We don’t need to do it in this PR either, or even in 2.3.0 (since adding a default is entirely backward compatible).

@Lee-W
Copy link
Member Author

Lee-W commented Mar 9, 2026

We don’t need to do it in this PR either, or even in 2.3.0 (since adding a default is entirely backward compatible).

yep, the issue I'm now wondering is whether timezone is handled correctly in the backfilled partitioned Dag run. Will fix this first and see whether I can resolve the thing you commented

@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch 2 times, most recently from dbd2f63 to c673b6d Compare March 16, 2026 09:36
@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch 2 times, most recently from 6b3c415 to 12d4a34 Compare March 27, 2026 07:09
@Lee-W Lee-W force-pushed the make-temporal-mapper-timezone-aware branch from 12d4a34 to db5fbd9 Compare March 27, 2026 07:58
@Lee-W Lee-W marked this pull request as ready for review March 27, 2026 08:03
@Lee-W Lee-W added this to the Airflow 3.2.0 milestone Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants