You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/09 19:56:35 UTC

[GitHub] [airflow] kaxil commented on a change in pull request #15293: Fix access_control syncing; faster cli sync-perm

kaxil commented on a change in pull request #15293:
URL: https://github.com/apache/airflow/pull/15293#discussion_r610873593



##########
File path: airflow/www/security.py
##########
@@ -516,24 +515,25 @@ def _get_all_roles_with_permissions(self) -> Dict[str, Role]:
 
     def create_dag_specific_permissions(self) -> None:
         """
-        Creates 'can_read' and 'can_edit' permissions for all active and paused DAGs.
+        Creates 'can_read' and 'can_edit' permissions for all active and paused DAGs,
+        along with any `access_control` permissions provided in the DAG.
 
         :return: None.
         """
         perms = self.get_all_permissions()
-        rows = (
-            self.get_session.query(models.DagModel.dag_id)
-            .filter(or_(models.DagModel.is_active, models.DagModel.is_paused))
-            .all()
-        )
+        dagbag = DagBag(read_dags_from_db=True)
+        dagbag.collect_dags_from_db()

Review comment:
       ooff -- This will load all the Serialized DAGs though here and start again with an incremental DagBag in https://github.com/apache/airflow/blob/9dd14aae40f4c2164ce1010cd5ee67d2317ea3ea/airflow/www/extensions/init_dagbag.py#L24-L32
   
   The Serialized DAG will be loaded when required by:
   
   https://github.com/apache/airflow/blob/9dd14aae40f4c2164ce1010cd5ee67d2317ea3ea/airflow/models/dagbag.py#L184-L200
   
   Let's say if someone changes the DAG File with a change in access_control and the Parsing process writes the `serialized_dag`, it will hit the above code-block and if 10 seconds have passed (`AIRFLOW__CORE__MIN_SERIALIZED_DAG_FETCH_INTERVAL` -https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#min-serialized-dag-fetch-interval), it will re-fetch and update the DAG. I think we should refresh the permission for that DAG in that nested if statement.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org