bujjibabukatta opened a new pull request, #71480:
URL: https://github.com/apache/airflow/pull/71480
Closes #71187.
### Problem
`[scheduler] create_cron_data_intervals` restores Airflow 2-style
`logical_date`/data-interval
semantics for `CronDataIntervalTimetable` and `DeltaDataIntervalTimetable`,
but it doesn't extend
to `run_id`. `generate_run_id()` is only defined once, on the shared base
`Timetable` class, and
always anchors to `run_after` — the AIP-76 default. So enabling the flag
restores `logical_date`
but silently leaves `run_id` on the new scheme, which breaks any workflow
that parses `run_id` to
recover the logical date (a common pattern carried over from Airflow 2).
This is a documented gap, not a regression — `_create_timetable()` in
`airflow.sdk.definitions.dag` confirms `_DataIntervalTimetable` subclasses
are only instantiated
when 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 `_DataIntervalTimetable`
base in
`airflow-core/src/airflow/timetables/interval.py`, covering both
`CronDataIntervalTimetable` and
`DeltaDataIntervalTimetable` symmetrically. It anchors to
`data_interval.start` when a data
interval is available, and falls back to the base (`run_after`-anchored)
behavior otherwise, so it
never raises on an unexpected `None`.
`CronTriggerTimetable`/`DeltaTriggerTimetable` (the AIP-76
default path) are untouched.
This mirrors the `LogicalDateRunIdTimetable` workaround already posted in
the issue by the
reporter, 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_id` anchors
to `data_interval.start`
for both `CronDataIntervalTimetable` and `DeltaDataIntervalTimetable`
- `test_generate_run_id_falls_back_without_data_interval` — falls back
correctly when
`data_interval=None`
- `test_generate_run_id_default_timetable_unaffected` — confirms
`CronTriggerTimetable` (default)
is unaffected, i.e. zero regression to the majority code path
### Docs
- `config.yml`: extended the `create_cron_data_intervals` description to
note it now also covers
`run_id`.
- `authoring-and-scheduling/timetable.rst`: added a scoping note to the
`run_id`/`logical_date`
section — 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 on `run_id`
reflecting the logical date.
### Verification performed
- `python -m py_compile` on changed files
- `ruff check` / `ruff format --check` against the repo's actual
`pyproject.toml` — clean
- Installed the real dependency chain and imported the actual patched
classes (not stand-ins) to
confirm `run_id` anchoring, the `None` fallback, and that
`CronTriggerTimetable` is unaffected —
all as expected
- Best-effort `mypy` (couldn't install the internal `airflow_mypy` plugin
outside CI, so treat as a
strong signal, not equivalent to the CI job)
- **Not run**: the full pytest suite via Breeze (`tests/conftest.py` shells
out to `uv` for
provider dependency metadata, which needs the Breeze/CI environment) —
please run
`breeze testing tests
airflow-core/tests/unit/timetables/test_interval_timetable.py` before
merging
### Open question for maintainers
The issue outlines two possible directions: (1) extend this flag to cover
`run_id` (what this PR
does), or (2) a more general `dag_run_policy`-style cluster policy hook,
noted in the issue as
possibly 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_intervals` is
documented in `config.yml` and listed in the CLI config commands, but I
couldn't find anywhere in
`_create_timetable()` (or elsewhere) that actually reads it to gate
`DeltaDataIntervalTimetable`
selection — `create_cron_data_intervals` appears 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.
--
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]
