Copilot commented on code in PR #64034:
URL: https://github.com/apache/airflow/pull/64034#discussion_r3025346406
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid.py:
##########
@@ -332,38 +323,74 @@ def get_grid_runs(
return_total_entries=False,
)
results = session.execute(dag_runs_select_filter).unique().all()
+ dag_runs = [run for run, _ in results]
+ attach_dag_versions_to_runs(dag_runs, session=session)
grid_runs = []
for run, has_missed in results:
- run.has_missed_deadline = has_missed
- grid_runs.append(GridRunsResponse.model_validate(run,
from_attributes=True))
+ grid_runs.append(
+ GridRunsResponse.model_validate(
+ {
+ "dag_id": run.dag_id,
+ "run_id": run.run_id,
+ "queued_at": run.queued_at,
+ "start_date": run.start_date,
+ "end_date": run.end_date,
+ "run_after": run.run_after,
+ "state": run.state,
+ "run_type": run.run_type,
+ "dag_versions": run.dag_versions,
+ "has_missed_deadline": has_missed,
Review Comment:
`dag_versions` is populated with `DagVersion` ORM objects (via
`attach_dag_versions_to_runs`). `DagVersionResponse` validation requires
`dag_model.dag_display_name` (AliasPath), but `attach_dag_versions_to_runs`
currently loads only `DagVersion.bundle`. For runs without `bundle_version`,
this can trigger extra lazy-load queries (N+1 per distinct dag_version) during
response serialization. Consider eager-loading `DagVersion.dag_model`
(optionally `load_only` the display-name columns) in the prefetch helper so
serialization doesn’t issue additional queries.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_grid.py:
##########
@@ -641,6 +641,29 @@ def test_get_grid_runs(self, session, test_client):
assert response.status_code == 200
assert _strip_dag_version_ids(response.json()) == [GRID_RUN_1,
GRID_RUN_2]
+ def test_get_grid_runs_multiple_dag_versions(self, session, test_client):
+ latest_dag_version =
session.scalar(select(DagModel).where(DagModel.dag_id ==
DAG_ID_5)).dag_versions[
+ -1
+ ]
+ latest_task_instance = session.scalar(
+ select(TaskInstance)
+ .where(TaskInstance.dag_id == DAG_ID_5, TaskInstance.run_id ==
"run_5_2")
+ .limit(1)
+ )
+ latest_task_instance.dag_version = latest_dag_version
Review Comment:
This test mutates a `TaskInstance` in `run_5_2` to the *latest* dag version,
but `run_5_2` is created after the second serialization and is likely already
using the latest `dag_version_id`. In that case the mutation is a no-op and the
test won’t actually exercise the “multiple dag versions per run” scenario. To
make the test robust, explicitly set one TI to an older dag version (e.g.
version 1) while leaving another TI on the newer version, and make the TI
selection deterministic (e.g. `order_by(TaskInstance.task_id)`).
```suggestion
dag_model = session.scalar(select(DagModel).where(DagModel.dag_id ==
DAG_ID_5))
# Ensure we have at least two distinct dag versions to work with.
older_dag_version = dag_model.dag_versions[0]
latest_dag_version = dag_model.dag_versions[-1]
# Deterministically pick task instances from run_5_2 and assign
different dag versions
task_instances = list(
session.scalars(
select(TaskInstance)
.where(TaskInstance.dag_id == DAG_ID_5, TaskInstance.run_id
== "run_5_2")
.order_by(TaskInstance.task_id)
)
)
# Make sure we have at least two TIs to assign different versions to.
assert len(task_instances) >= 2
task_instances[0].dag_version = older_dag_version
task_instances[-1].dag_version = latest_dag_version
```
--
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]
