jason810496 commented on issue #57966:
URL: https://github.com/apache/airflow/issues/57966#issuecomment-3578725267
Thanks everyone for the discussion.
> I also like the endpoint that @pierrejeambrun proposed - it is exactly how
Auth Manager has been designed. In Auth Manager design we made a DELIBERATE
decision that we delegate roles, groups and other decision making to the Auth
Manager. Aiflow just wants to do very simple Auth mechanism:
Yes, very agree as well!
However, I have some concern about
1. How large will the scope to be supported in the future?
- Do we just need to support `DagAccessEntity.RUN` access entiry for now?
- Or do we need to support all the access entiry that visiable on the UI?
2. Generic batch authorize method support for BaseAuthManger and also for
each providers
- It seems we need to introduce a generic `batch_is_authorized_entites`
on Auth Manager side to avoid the N+1 query or API call probelm, as the new
meta permission endpoint will be dedicate UI route that will be called for
every page, and the latency will be significant increase if the `page_size` is
large (not really sure about the lifecycle of API calls and UI rendering, but
it seems to effect the UX)
- Take `batch_is_authorized_dag` as example, only `AwsAuthManager` had
implemented the *real* batch method, the rest of the providers will still
enounter the N+1 problem I mentioned above.
-
https://github.com/apache/airflow/blob/13dc31fc065d0250229bc38b0425bdd1bddcf9a9/providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py#L271-L272
-
https://github.com/apache/airflow/blob/13dc31fc065d0250229bc38b0425bdd1bddcf9a9/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py#L375-L376
--
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]
