uranusjr commented on a change in pull request #19130:
URL: https://github.com/apache/airflow/pull/19130#discussion_r733934959
##########
File path: airflow/timetables/interval.py
##########
@@ -81,9 +81,15 @@ def next_dagrun_info(
return None
start = self._align(earliest)
else:
- # There's a previous run. Create a data interval starting from when
- # the end of the previous interval.
- start = last_automated_data_interval.end
+ # There's a previous run.
+ if earliest is not None:
+ # Catchup is False or DAG has new start date in the future.
+ # Make sure we get the latest start date
+ start = max(last_automated_data_interval.end, earliest)
+ else:
+ # Create a data interval starting from when the end of the
previous interval.
+ start = last_automated_data_interval.end
Review comment:
Could you move the first comment to the end of the `else:`? Like this
```python
else: # There's a previous run.
```
Because right now it isn't very clear whether this comment is describing the
`else`, or the `if earliest is not None:` block below.
Otherwise this looks good to me. Could you add a test in `tests/timetables`
for this? There's already a file in there, and you should be able to set up
cases roughly following its structure to test for the `catchup`, `start_date`,
and `last_automated_data_interval` cases (so 8 in total) with
`pytest.mark.parametrize`.
--
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]
