jens-scheffler-bosch commented on code in PR #30706:
URL: https://github.com/apache/airflow/pull/30706#discussion_r1187949766
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1308,8 +1308,18 @@ def _create_dag_runs_dataset_triggered(
DatasetDagRunQueue.target_dag_id == dag_run.dag_id
).delete()
- def _should_update_dag_next_dagruns(self, dag, dag_model: DagModel,
total_active_runs: int) -> bool:
+ def _should_update_dag_next_dagruns(
+ self, dag: DAG, dag_model: DagModel, session: Session,
total_active_runs: int = -1
+ ) -> bool:
"""Check if the dag's next_dagruns_create_after should be updated."""
+ # If dag has no interval or timetable then skip save runtime
+ if not dag.schedule_interval and isinstance(dag.timetable,
NullTimetable):
+ return False
Review Comment:
Okay, thanks for the feedback. I am thinking about how to apply the flag w/o
adding more complexity in attributes. Do you think rather (1) we should add a
new attribute just for this use case to the `Timetable` class? I was thinking
but can not imagine a good self-descriptive name other than `can run`. So (2)
would it be probably meaningful to change the default of `can_run`only on
`NullTimetable`and `OnceTimetable`? Because actually when taking a look to the
code it is rather mis-placed in `ContinuousTimetable` - but I am not sure what
other side effects might be?
Mhm, especially when looking at `airflow/models/dag.py:3125` I feel like
`can_run` is not correct in validation for `ContinuousTimetable`.
--
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]
