Dev-iL commented on code in PR #73966:
URL: https://github.com/apache/airflow/pull/73966#discussion_r4155299065
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -1336,10 +1336,10 @@ def get_task_instance_states(
if map_index is not None:
query = query.where(TI.map_index == map_index)
- results = session.scalars(query).all()
+ results = (await session.scalars(query)).all()
Review Comment:
To clarify what I meant by "a regression for event-loop availability":
please don't assert on timings, they'd be flaky in CI. A deterministic proxy
works better: count the `TaskInstance` entities the ORM builds during the
request. The current query builds one per returned row, and the column select
builds none.
Something like:
```python
@pytest.fixture
def task_instance_loads():
loads = []
def on_load(target, context):
loads.append(target)
event.listen(TaskInstance, "load", on_load)
yield loads
event.remove(TaskInstance, "load", on_load)
def test_states_does_not_hydrate_task_instances(client, dag_maker, session,
task_instance_loads):
... # seed one run with a few mapped instances, commit,
session.expunge_all()
task_instance_loads.clear()
response = client.get("/execution/task-instances/states", params={...})
assert response.status_code == 200
assert len(response.json()["task_states"][run_id]) == expected
assert task_instance_loads == []
```
I tried this against the current full-entity query: it records 11 loads for
10 mapped instances and 201 for 200, identical across repeated runs, while the
four-column select records 0. A few rows are enough, since the count is exact.
Please also seed a task group and pass `task_group_id`, so the same
assertion covers `_get_group_tasks` (line 1411), which selects full entities
too.
--
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]
