pierrejeambrun commented on code in PR #67823:
URL: https://github.com/apache/airflow/pull/67823#discussion_r3459974487
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py:
##########
@@ -48,11 +60,7 @@
status.HTTP_404_NOT_FOUND,
]
),
- dependencies=[
- Depends(requires_access_dag("GET")),
- Depends(requires_access_dag("GET", DagAccessEntity.DEPENDENCIES)),
- Depends(requires_access_dag("GET", DagAccessEntity.TASK_INSTANCE)),
- ],
+ dependencies=[Depends(check_structure_data_access)],
Review Comment:
Could we keep the unconditional `Depends(requires_access_dag("GET"))` here
directly in `dependencies` (declaratively, like the other UI routes), and move
only the conditional DEPENDENCIES check into a helper — and rename that helper
to something narrower like `require_dependencies_access_when_requested`? That
way the unconditional check stays visible at the route surface and the helper
only owns the conditional bit:
```python
dependencies=[
Depends(requires_access_dag("GET")),
Depends(require_dependencies_access_when_requested),
],
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/structure.py:
##########
@@ -40,6 +40,18 @@
structure_router = AirflowRouter(tags=["Structure"], prefix="/structure")
+def check_structure_data_access(
+ request: Request,
+ user: GetUserDep,
+ external_dependencies: bool = False,
+) -> None:
+ """Check permissions for the structure endpoint."""
+ requires_access_dag("GET")(request, user)
Review Comment:
Coming back to my prior question — the `DagAccessEntity.TASK_INSTANCE` check
has been dropped entirely (was in the old `dependencies=[...]` block, no longer
anywhere). Could you confirm and document why? My read is it's correct since
`structure_data` returns the static task graph + asset edges, not TI runtime
state — but this is a permissions change so it'd help to have a one-line note
in the PR body (or as a comment on the helper) explaining the rationale, so a
reviewer six months from now isn't left wondering whether dropping TI access
was intentional or a regression.
--
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]
