pierrejeambrun commented on code in PR #76583:
URL: https://github.com/apache/airflow/pull/76583#discussion_r4491888863
##########
airflow-core/src/airflow/api_fastapi/core_api/security.py:
##########
@@ -648,7 +651,9 @@ async def inner(
)
# Select the id alongside dag_id: a NULL dag_id and a missing row
both come back
# as None from a bare ``Log.dag_id`` scalar, and they authorize
differently.
- row = session.execute(select(Log.id, Log.dag_id).where(Log.id ==
event_log_id)).one_or_none()
+ row = (
+ await session.execute(select(Log.id, Log.dag_id).where(Log.id
== event_log_id))
Review Comment:
Further down, when the row has a `dag_id`, this still ends up in
`DagModel.get_team_name` through `requires_access_dag`, which is a sync query
on the event loop whenever the team-name cache misses. Same for `GET
/eventLogs?dag_id=...`. With the cache TTL at 0 and a 2-connection pool, 40
concurrent requests for a row with a Dag gave me 31 timeouts and 5 500s (rows
without a Dag were fine). Could this use `DagModel.get_team_name_async` from
#76493 and authorize with `DagDetails(id=dag_id, team_name=team_name)`
directly, like that PR does for backfills? A route test that seeds a `Log` row
with a `dag_id` would probably be worth adding too, since the #76424 sweep runs
against an empty DB and never reaches this branch.
--
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]
