norm commented on code in PR #23510:
URL: https://github.com/apache/airflow/pull/23510#discussion_r866789187
##########
airflow/security/permissions.py:
##########
@@ -66,14 +66,16 @@
DAG_ACTIONS = {ACTION_CAN_READ, ACTION_CAN_EDIT, ACTION_CAN_DELETE}
-def resource_name_for_dag(dag_id):
+def resource_name_for_dag(dag_id, is_subdag=False):
"""Returns the resource name for a DAG id."""
if dag_id == RESOURCE_DAG:
return dag_id
if dag_id.startswith(RESOURCE_DAG_PREFIX):
return dag_id
- # To account for SubDags
- root_dag_id = dag_id.split(".")[0]
- return f"{RESOURCE_DAG_PREFIX}{root_dag_id}"
+ if is_subdag:
+ # To account for SubDags
+ root_dag_id = dag_id.split(".")[0]
Review Comment:
What if this **is** a subdag and the dag has a `.` in the name? What if the
dag and the subdag **both** have `.`s in their names?
##########
airflow/www/security.py:
##########
@@ -340,7 +340,15 @@ def get_accessible_dag_ids(self, user, user_actions=None,
session=None) -> Set[s
def can_access_some_dags(self, action: str, dag_id: Optional[str] = None)
-> bool:
"""Checks if user has read or write access to some dags."""
if dag_id and dag_id != '~':
- return self.has_access(action,
permissions.resource_name_for_dag(dag_id))
+ is_subdag = False
Review Comment:
This code block is repeated several times, could it be abstracted?
--
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]
