ashb commented on a change in pull request #10956:
URL: https://github.com/apache/airflow/pull/10956#discussion_r499882907
##########
File path: airflow/models/dag.py
##########
@@ -1573,34 +1680,28 @@ def create_dagrun(self,
@classmethod
@provide_session
- def bulk_sync_to_db(cls, dags: Collection["DAG"], sync_time=None,
session=None):
+ def bulk_sync_to_db(cls, dags: Collection["DAG"], session=None):
Review comment:
I see this as "synchronize the state of the DB with the dags [from the
dagbag]" so to me this name accurately reflect the what this method does.
And because of transactions/locking, and wanting to avoid n+1 queries, the
best we could do is pull out a method like below, which would still have to be
called from within bulk_sync_to_db:
```python
def _calculate_dagrun_date_fields(self, dag, most_recent_dag_run,
active_runs_of_dag):
self.next_dagrun, self.next_dagrun_create_after =
dag.next_dagrun_info(
most_recent_dag_run
)
if dag.max_active_runs and active_runs_of_dag >= dag.max_active_runs:
# Since this happens every time the dag is parsed it would be
quite spammy at info
log.debug(
"DAG %s is at (or above) max_active_runs (%d of %d), not
creating any more runs",
dag.dag_id, active_runs_of_dag, dag.max_active_runs
)
self.next_dagrun_create_after = None
log.info("Setting next_dagrun for %s to %s", dag.dag_id,
self.next_dagrun)
```
Called like this:
```python
orm_dag._calculate_dagrun_date_fields(
most_recent_dag_runs.get(dag.dag_id),
num_active_runs.get(dag.dag_id, 0)
)
```
Is that the sort of thing you were thinking of?
(Oh except, we can't make it if it's on DagModel as we are calling from
within DAG)
----------------------------------------------------------------
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]
