Repository navigation
Anchor run_id to logical_date when create_cron_data_intervals is enab… - #71480
bujjibabukatta wants to merge 1 commit into
Conversation
|
Please refer to the original issue for discussions. |
Thank you for the pointer. I've reviewed the original issue #71187 and implemented the fix with the correct scope. When enabled: CronDataIntervalTimetable / DeltaDataIntervalTimetable Moved generate_run_id override only to CronDataIntervalTimetable — this correctly limits the behavior to cron expressions when the flag is enabled> - DeltaDataIntervalTimetable (used for @daily, @hourly, timedelta()) now correctly retains AIP-76 run_after anchoring> - Added test test_generate_run_id_uses_run_after_for_delta_and_default_cron to verify this> - CronTriggerTimetable (AIP-76 default) remains unaffected as verified by test_generate_run_id_default_timetable_unaffected |
7452fed to
ffa3e7e
Compare
Closes #71187.
Problem
[scheduler] create_cron_data_intervalsrestores Airflow 2-stylelogical_date/data-intervalsemantics for
CronDataIntervalTimetableandDeltaDataIntervalTimetable, but it doesn't extendto
run_id.generate_run_id()is only defined once, on the shared baseTimetableclass, andalways anchors to
run_after— the AIP-76 default. So enabling the flag restoreslogical_datebut silently leaves
run_idon the new scheme, which breaks any workflow that parsesrun_idtorecover the logical date (a common pattern carried over from Airflow 2).
This is a documented gap, not a regression —
_create_timetable()inairflow.sdk.definitions.dagconfirms_DataIntervalTimetablesubclasses are only instantiatedwhen the flag is on, so the fix is naturally scoped by construction rather than an extra
conditional.
Fix
Adds a
generate_run_id()override on the shared_DataIntervalTimetablebase inairflow-core/src/airflow/timetables/interval.py, covering bothCronDataIntervalTimetableandDeltaDataIntervalTimetablesymmetrically. It anchors todata_interval.startwhen a datainterval is available, and falls back to the base (
run_after-anchored) behavior otherwise, so itnever raises on an unexpected
None.CronTriggerTimetable/DeltaTriggerTimetable(the AIP-76default path) are untouched.
This mirrors the
LogicalDateRunIdTimetableworkaround already posted in the issue by thereporter, who confirmed it works in their environment — promoted here into core so it applies
without a custom DAG-level subclass.
Tests
Added three regression tests to
airflow-core/tests/unit/timetables/test_interval_timetable.py:test_generate_run_id_anchors_to_data_interval_start—run_idanchors todata_interval.startfor both
CronDataIntervalTimetableandDeltaDataIntervalTimetabletest_generate_run_id_falls_back_without_data_interval— falls back correctly whendata_interval=Nonetest_generate_run_id_default_timetable_unaffected— confirmsCronTriggerTimetable(default)is unaffected, i.e. zero regression to the majority code path
Docs
config.yml: extended thecreate_cron_data_intervalsdescription to note it now also coversrun_id.authoring-and-scheduling/timetable.rst: added a scoping note to therun_id/logical_datesection — that section previously described this behavior as already true, which it wasn't until
this PR.
installation/upgrading_to_airflow3.rst: added a note for 2→3 migrators who rely onrun_idreflecting the logical date.
Verification performed
python -m py_compileon changed filesruff check/ruff format --checkagainst the repo's actualpyproject.toml— cleanconfirm
run_idanchoring, theNonefallback, and thatCronTriggerTimetableis unaffected —all as expected
mypy(couldn't install the internalairflow_mypyplugin outside CI, so treat as astrong signal, not equivalent to the CI job)
tests/conftest.pyshells out touvforprovider dependency metadata, which needs the Breeze/CI environment) — please run
breeze testing tests airflow-core/tests/unit/timetables/test_interval_timetable.pybeforemerging
Open question for maintainers
The issue outlines two possible directions: (1) extend this flag to cover
run_id(what this PRdoes), or (2) a more general
dag_run_policy-style cluster policy hook, noted in the issue aspossibly the preferred long-term design. This PR takes the smaller-scoped option since it's a pure
addition with low regression risk, but happy to rework toward the hook approach if that's the
preferred direction.
One incidental finding while researching this:
[scheduler] create_delta_data_intervalsisdocumented in
config.ymland listed in the CLI config commands, but I couldn't find anywhere in_create_timetable()(or elsewhere) that actually reads it to gateDeltaDataIntervalTimetableselection —
create_cron_data_intervalsappears to gate both cron and delta branches today.Flagging in case that's a known issue or I'm missing a call site; didn't want to fix it as a
drive-by in this PR.
Newsfragment will be added as a follow-up commit once this PR has a number, per the contribution
guide.
AI disclosure: All code, documentation, and test changes in this PR were written by me. An AI
coding tool (Claude) was used only PR description and to run the new regression test cases locally and verify they
pass against the patched code.