ashb commented on code in PR #73692:
URL: https://github.com/apache/airflow/pull/73692#discussion_r4103264446
##########
airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml:
##########
@@ -13511,6 +13514,7 @@ components:
- timezone
- last_parsed
- default_args
+ - exceeds_max_active_runs
Review Comment:
Should this be required, or only sent when it's true? (Not present being
implicitly false)
##########
airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts:
##########
@@ -3285,6 +3285,10 @@ export const $DAGDetailsResponse = {
title: 'Active Runs Count',
default: 0
},
+ exceeds_max_active_runs: {
+ type: 'boolean',
+ title: 'Exceeds Max Active Runs'
Review Comment:
Isn't "exceeds" wrong? Since this will be true when it's at or above max
active runs? and "at" isn't exceeding, but it still won't schedule new dag runs.
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -176,9 +176,19 @@ def calculate(cls, dag: LazyDeserializedDAG, *, session:
Session) -> Self:
:param dags: dict of dags to query
"""
- # Skip these queries entirely if no Dags can be scheduled to save time.
+ active_run_counts = DagRun.active_runs_of_dags(
Review Comment:
At the very least, we should only do this query when the dag defines
max_active_runs.
Further though, I'm worried about this beocming an n+1 query and tanking
performance.
--
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]
