pierrejeambrun commented on code in PR #60801:
URL: https://github.com/apache/airflow/pull/60801#discussion_r2707667921
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py:
##########
@@ -79,11 +79,6 @@ def get_import_error(
session.expunge(error)
auth_manager = get_auth_manager()
- can_read_all_dags = auth_manager.is_authorized_dag(method="GET", user=user)
- if can_read_all_dags:
- # Early return if the user has access to all DAGs
- return error
Review Comment:
Yeah I also liked the early returns for massive dag ids list. But I don't
think we have the equivalent for 'read all dags'.
The `batch_is_authorized_dag` shouldn't be too costly.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py:
##########
@@ -79,11 +79,6 @@ def get_import_error(
session.expunge(error)
auth_manager = get_auth_manager()
- can_read_all_dags = auth_manager.is_authorized_dag(method="GET", user=user)
- if can_read_all_dags:
- # Early return if the user has access to all DAGs
- return error
-
readable_dag_ids = auth_manager.get_authorized_dag_ids(user=user)
Review Comment:
Don't we have the same problem here? We will create a CTE with ids that
correspond to dag we can 'list' but not to dags we can 'read' details from.
--
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]
