ashb commented on a change in pull request #4867: [AIRFLOW-3831] Remove DagBag
from /pickle_info
URL: https://github.com/apache/airflow/pull/4867#discussion_r263770115
##########
File path: airflow/www/views.py
##########
@@ -466,17 +466,36 @@ def show_traceback(self):
@expose('/pickle_info')
@has_access
def pickle_info(self):
- d = {}
- filter_dag_ids = appbuilder.sm.get_accessible_dag_ids()
- if not filter_dag_ids:
+ """Return pickle information for given DAG ids."""
+ allowed_dag_ids = appbuilder.sm.get_accessible_dag_ids()
+ if not allowed_dag_ids:
return wwwutils.json_response({})
+
dag_id = request.args.get('dag_id')
- dags = [dagbag.dags.get(dag_id)] if dag_id else dagbag.dags.values()
- for dag in dags:
- if 'all_dags' in filter_dag_ids or dag.dag_id in filter_dag_ids:
- if not dag.is_subdag:
- d[dag.dag_id] = dag.pickle_info()
- return wwwutils.json_response(d)
+ # Check if dag_id is allowed to be accessed
+ if dag_id and allowed_dag_ids != {"all_dags"} and dag_id not in
allowed_dag_ids:
Review comment:
Rather than leaking the implementation of the security manage out to the
views the `@has_dag_access` decorator should be used I think.
And/or the has_all_dag_access method.
Or if we need to do some extra filtering (i.e. dag_id) is optional then a
new method decorator.
But I don't want to leak the SM implementation detail of `{"all_dags"}`
outside of the security manager.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
