jedcunningham commented on code in PR #66132:
URL: https://github.com/apache/airflow/pull/66132#discussion_r3169866432
##########
airflow-core/tests/unit/models/test_dag.py:
##########
@@ -623,7 +623,7 @@ def test_bulk_write_to_db_max_active_runs(self,
testing_dag_bundle, state, catch
logical_date=model.next_dagrun,
run_type=DagRunType.SCHEDULED,
session=session,
- data_interval=(model.next_dagrun, model.next_dagrun),
+ data_interval=(model.next_dagrun, model.next_dagrun +
timedelta(days=1)),
Review Comment:
Thanks — agreed.
##########
airflow-core/src/airflow/timetables/interval.py:
##########
@@ -110,6 +110,15 @@ def next_dagrun_info(
else:
# Data interval starts from the end of the previous interval.
start = align_last_data_interval_end
+ # A zero-length previous interval (start == end) collapses
logical_date
+ # onto the existing run; advance one period so we don't re-emit it.
+ # This shape comes from CronTriggerTimetable runs scheduled before
a
+ # switch to a CronDataIntervalTimetable.
+ if (
+ last_automated_data_interval.start ==
last_automated_data_interval.end
+ and start == last_automated_data_interval.start
+ ):
+ start = self._get_next(start)
Review Comment:
The new interval is `(start, _get_next(start))`, so for it to be zero-length
`_get_next` would have to return its argument unchanged. That can't happen for
either `_DataIntervalTimetable` subclass:
- `CronDataIntervalTimetable._get_next` uses `croniter`, which always
returns the strictly-later boundary.
- `DeltaDataIntervalTimetable._get_next` returns `current + delta`, and
`DeltaMixin.validate()` (`_delta.py:58-59`) rejects `delta <= 0`.
So a single advance is sufficient — no looping needed.
--
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]
