harjothkhara opened a new pull request, #70089:
URL: https://github.com/apache/airflow/pull/70089
## Why
`CronMixin._get_prev` can return a tick that is not strictly earlier than
the time it is asked about, so a manually triggered run gets a bad data
interval on the two days a year the clocks change.
With `CronDataIntervalTimetable("0 2 * * *", timezone="America/New_York")`,
a manual run at `2024-03-10 07:30Z`:
```
data interval: 2024-03-10 07:00Z -> 2024-03-10 07:00Z (zero length)
```
The scheduled path returns the correct `2024-03-09 07:00Z -> 2024-03-10
07:00Z` for the same period, so the two paths disagree about the same run.
croniter steps back on naive wall-clock time, but `make_aware` maps a tick
that falls in a spring-forward gap onto the next real instant. When the
previous tick is in the gap it lands on `current`, and `_get_prev` returns
`current` itself.
The same remapping overshoots on the fall-back side. With `0 1 * * *` in
`America/New_York` at `2024-11-03 05:30Z`, `_get_prev` returns `06:00Z` — half
an hour in the future — and the inferred interval ends after `run_after`.
`CronTriggerTimetable` with `run_immediately=True` picks that future tick as
the run to fire now.
The zero-length interval has a second effect. `next_dagrun_info` treats a
zero-length previous interval as a point-in-time run and advances one period
(#66132, which is correct for the `CronTriggerTimetable` case it was written
for). Fed this interval it skips a whole day:
```
after the buggy interval : 2024-03-11 06:00Z -> 2024-03-12 06:00Z
if the interval is right : 2024-03-10 07:00Z -> 2024-03-11 06:00Z
```
Also reproduces with `Europe/Zurich` `0 2 * * *` at 2023-03-26, and in
`Australia/Sydney` and `Australia/Lord_Howe`.
## What
- `_get_prev` keeps stepping `cron.get_prev` until the converted result is
strictly earlier than `current`. `get_prev` is strictly decreasing on naive
time, so it terminates.
- The `_covers_every_hour` branch is left alone. It computes a wall-clock
delta rather than going through `make_aware`, and the delta is strictly
positive, so it cannot produce this.
- Docstring now states the strictly-earlier guarantee that callers already
assume.
Over 23,040 sampled (cron, timezone, instant) combinations across eight
timezones, 31 results change: 17 in `_get_prev`, 12 in `_align_to_prev`, 2 in
`_align_to_next`. In every one the old value was already wrong — at or after
the instant asked about, or a period skipped. Nothing outside a DST transition
window changes.
I also checked `_get_next` for the mirror problem and did not find one — it
never returned a value at or before its input across 7,680 sampled instants
around transitions. It does skip the second occurrence of an ambiguous hour,
but that is the existing `fold=1` policy in `make_aware` and is what
`test_fold_scheduling` expects, so I left it out.
## Testing
Three regression tests, all failing before the change and passing after:
- `test_cron_infer_manual_data_interval_dst` — gap and fold for
`America/New_York`, `Europe/Zurich` and `Australia/Lord_Howe` (a 30-minute
offset). Asserts the interval is non-empty and does not end after `run_after`.
- `test_cron_next_dagrun_info_after_dst_manual_run` — the skipped period.
- `test_cron_trigger_run_immediately_does_not_pick_future_run` —
`CronTriggerTimetable`, which shares the mixin.
`airflow-core/tests/unit/timetables/` is 246 passed.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Opus 4.8 following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
