uranusjr commented on a change in pull request #15397:
URL: https://github.com/apache/airflow/pull/15397#discussion_r639193579
##########
File path: airflow/models/dag.py
##########
@@ -533,94 +537,54 @@ def next_dagrun_info(
"automated" DagRuns for this dag (scheduled or backfill, but not
manual)
"""
- if (
- self.schedule_interval == "@once" and date_last_automated_dagrun
- ) or self.schedule_interval is None:
- # Manual trigger, or already created the run for @once, can short
circuit
+ # XXX: The timezone.coerce_datetime calls in this function should not
+ # be necessary since the function annotation suggests it only accepts
+ # pendulum.DateTime, and someone is passing datetime.datetime into this
+ # function. We should fix whatever is doing that.
+ if self.is_subdag:
return (None, None)
- next_execution_date =
self.next_dagrun_after_date(date_last_automated_dagrun)
-
- if next_execution_date is None:
+ time_table: TimeTable = self.time_table
+ restriction = self._format_time_restriction()
+ if not self.catchup:
+ restriction = time_table.cancel_catchup(restriction)
Review comment:
The thing I don’t particular like about this is `catchup` does not make
sense for any runs after the first, which to me indicates the design isn’t
right. If we want to make catchup handling a part of the time table, a better
interface to me would be to separate the first and subsequent DAG runs:
```python
def first_dagrun_info(
self,
between: TimeRestriction,
catchup: bool,
) -> Optional[DagRunInfo]:
...
def next_dagrun_info(
self,
last_automated_dagrun: DateTime,
between: TimeRestriction,
) -> Optional[DagRunInfo]:
...
```
This especially makes sense to me because all current time table
implementations all contain a clear logical distinction between the first and
sebsequent DAG runs, also indicating mayne they should be different functions.
The worry though is maybe in the future some time table may somehow have its
non-first DAG run depend on the catchup configuration? But I have difficulties
coming up with a sane example.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
