From 6f281e68000dab706ddeba375ad1f304b3d5bd30 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Fri, 24 Dec 2021 15:47:59 +0100 Subject: [PATCH 1/6] Webserver - Change URL routes for DAG page and rename "tree" to "grid" view - Rename tree view to grid view - Updated test cases for new paths - Update URL routes and add redirects --- airflow/www/templates/airflow/dag.html | 20 ++-- airflow/www/views.py | 155 +++++++++++++++++++++---- tests/www/views/test_views_tasks.py | 75 +++++++++++- 3 files changed, 215 insertions(+), 35 deletions(-) diff --git a/airflow/www/templates/airflow/dag.html b/airflow/www/templates/airflow/dag.html index e57a1f1fc8b3b..594c4c1716e65 100644 --- a/airflow/www/templates/airflow/dag.html +++ b/airflow/www/templates/airflow/dag.html @@ -110,33 +110,33 @@

diff --git a/airflow/www/views.py b/airflow/www/views.py index b73530170bc87..2be6495a1c3bb 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1054,15 +1054,24 @@ def last_dagruns(self, session=None): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), ] ) + def code(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_code', **request.args)) + + @expose('/dags//code') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), + ] + ) @provide_session - def code(self, session=None): + def dag_code(self, dag_id, session=None): """Dag Code.""" all_errors = "" dag_orm = None - dag_id = None try: - dag_id = request.args.get('dag_id') dag_orm = DagModel.get_dagmodel(dag_id, session=session) code = DagCode.get_code_by_fileloc(dag_orm.fileloc) html_code = Markup(highlight(code, lexers.PythonLexer(), HtmlFormatter(linenos=True))) @@ -1095,10 +1104,20 @@ def code(self, session=None): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), ] ) + def dag_details_redirect(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_details', **request.args)) + + @expose('/dags//details') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), + ] + ) @provide_session - def dag_details(self, session=None): + def dag_details(self, dag_id, session=None): """Get Dag details.""" - dag_id = request.args.get('dag_id') dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) @@ -2301,6 +2320,20 @@ def success(self): State.SUCCESS, ) + @expose('/dags/') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG), + ] + ) + @gzipped + @action_logging + def dag(self, dag_id): + """Redirect to default DAG view.""" + return redirect(url_for('Airflow.dag_grid', dag_id=dag_id, **request.args)) + @expose('/tree') @auth.has_access( [ @@ -2311,10 +2344,23 @@ def success(self): ) @gzipped @action_logging + def tree(self): + """Redirect to the replacement - grid view.""" + return redirect(url_for('Airflow.dag_grid', **request.args)) + + @expose('/dags//grid') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG), + ] + ) + @gzipped + @action_logging @provide_session - def tree(self, session=None): - """Get Dag as tree.""" - dag_id = request.args.get('dag_id') + def dag_grid(self, dag_id, session=None): + """Get Dag as tree (grid).""" dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) if not dag: @@ -2399,8 +2445,21 @@ def tree(self, session=None): ) @gzipped @action_logging + def calendar(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_calendar', **request.args)) + + @expose('/dags//calendar') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + ] + ) + @gzipped + @action_logging @provide_session - def calendar(self, session=None): + def dag_calendar(self, dag_id, session=None): """Get DAG runs as calendar""" def _convert_to_date(session, column): @@ -2410,7 +2469,6 @@ def _convert_to_date(session, column): else: return func.date(column) - dag_id = request.args.get('dag_id') dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) if not dag: @@ -2476,10 +2534,23 @@ def _convert_to_date(session, column): ) @gzipped @action_logging + def graph(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_graph', **request.args)) + + @expose('/dags//graph') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG), + ] + ) + @gzipped + @action_logging @provide_session - def graph(self, session=None): + def dag_graph(self, dag_id, session=None): """Get DAG as Graph.""" - dag_id = request.args.get('dag_id') dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) if not dag: @@ -2569,11 +2640,22 @@ class GraphForm(DateTimeWithNumRunsWithDagRunsForm): ] ) @action_logging + def duration(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_duration', **request.args)) + + @expose('/dags//duration') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + ] + ) + @action_logging @provide_session - def duration(self, session=None): + def dag_duration(self, dag_id, session=None): """Get Dag as duration graph.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') - dag_id = request.args.get('dag_id') dag_model = DagModel.get_dagmodel(dag_id) dag: Optional[DAG] = current_app.dag_bag.get_dag(dag_id) @@ -2711,11 +2793,22 @@ def duration(self, session=None): ] ) @action_logging + def tries(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_tries', **request.args)) + + @expose('/dags//tries') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + ] + ) + @action_logging @provide_session - def tries(self, session=None): + def dag_tries(self, dag_id, session=None): """Shows all tries.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') - dag_id = request.args.get('dag_id') dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) base_date = request.args.get('base_date') @@ -2788,11 +2881,22 @@ def tries(self, session=None): ] ) @action_logging + def landing_times(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_landing_times', **request.args)) + + @expose('/dags//landing_times') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + ] + ) + @action_logging @provide_session - def landing_times(self, session=None): + def dag_landing_times(self, dag_id, session=None): """Shows landing times.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') - dag_id = request.args.get('dag_id') dag: DAG = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) base_date = request.args.get('base_date') @@ -2893,10 +2997,21 @@ def paused(self): ] ) @action_logging + def gantt(self): + """Redirect from url param.""" + return redirect(url_for('Airflow.dag_gantt', **request.args)) + + @expose('/dags//gantt') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + ] + ) + @action_logging @provide_session - def gantt(self, session=None): + def dag_gantt(self, dag_id, session=None): """Show GANTT chart.""" - dag_id = request.args.get('dag_id') dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py index d433542fd3264..3dc749fbbc852 100644 --- a/tests/www/views/test_views_tasks.py +++ b/tests/www/views/test_views_tasks.py @@ -139,16 +139,26 @@ def client_ti_without_dag_edit(app): pytest.param( 'dag_details?dag_id=example_bash_operator', ['DAG Details'], - id="dag-details", + id="dag-details-url-param", ), pytest.param( 'dag_details?dag_id=example_subdag_operator.section-1', ['DAG Details'], + id="dag-details-subdag-url-param", + ), + pytest.param( + 'dags/example_subdag_operator.section-1/details', + ['DAG Details'], id="dag-details-subdag", ), pytest.param( 'graph?dag_id=example_bash_operator', ['runme_1'], + id='graph-url-param', + ), + pytest.param( + 'dags/example_bash_operator/graph', + ['runme_1'], id='graph', ), pytest.param( @@ -156,34 +166,69 @@ def client_ti_without_dag_edit(app): ['runme_1'], id='tree', ), + pytest.param( + 'dags/example_bash_operator/grid', + ['runme_1'], + id='grid', + ), pytest.param( 'tree?dag_id=example_subdag_operator.section-1', ['section-1-task-1'], - id="tree-subdag", + id="tree-subdag-url-param", + ), + pytest.param( + 'dags/example_subdag_operator.section-1/grid', + ['section-1-task-1'], + id="grid-subdag", ), pytest.param( 'duration?days=30&dag_id=example_bash_operator', ['example_bash_operator'], + id='duration-url-param', + ), + pytest.param( + 'dags/example_bash_operator/duration?days=30', + ['example_bash_operator'], id='duration', ), pytest.param( 'duration?days=30&dag_id=missing_dag', ['seems to be missing'], + id='duration-missing-url-param', + ), + pytest.param( + 'dags/missing_dag/duration?days=30', + ['seems to be missing'], id='duration-missing', ), pytest.param( 'tries?days=30&dag_id=example_bash_operator', ['example_bash_operator'], + id='tries-url-param', + ), + pytest.param( + 'dags/example_bash_operator/tries?days=30', + ['example_bash_operator'], id='tries', ), pytest.param( 'landing_times?days=30&dag_id=example_bash_operator', ['example_bash_operator'], + id='landing-times-url-param', + ), + pytest.param( + 'dags/example_bash_operator/landing_times?days=30', + ['example_bash_operator'], id='landing-times', ), pytest.param( 'gantt?dag_id=example_bash_operator', ['example_bash_operator'], + id="gantt-url-param", + ), + pytest.param( + 'dags/example_bash_operator/gantt', + ['example_bash_operator'], id="gantt", ), pytest.param( @@ -196,21 +241,41 @@ def client_ti_without_dag_edit(app): pytest.param( "graph?dag_id=example_bash_operator", ["example_bash_operator"], + id="existing-dagbag-graph-url-param", + ), + pytest.param( + "dags/example_bash_operator/graph", + ["example_bash_operator"], id="existing-dagbag-graph", ), pytest.param( "tree?dag_id=example_bash_operator", ["example_bash_operator"], - id="existing-dagbag-tree", + id="existing-dagbag-tree-url-param", + ), + pytest.param( + "dags/example_bash_operator/grid", + ["example_bash_operator"], + id="existing-dagbag-grid", ), pytest.param( "calendar?dag_id=example_bash_operator", ["example_bash_operator"], + id="existing-dagbag-calendar-url-param", + ), + pytest.param( + "dags/example_bash_operator/calendar", + ["example_bash_operator"], id="existing-dagbag-calendar", ), pytest.param( "dag_details?dag_id=example_bash_operator", ["example_bash_operator"], + id="existing-dagbag-dag-details-url-param", + ), + pytest.param( + "dags/example_bash_operator/details", + ["example_bash_operator"], id="existing-dagbag-dag-details", ), pytest.param( @@ -348,7 +413,7 @@ def test_code_from_db(admin_client): dag = DagBag(include_examples=True).get_dag("example_bash_operator") DagCode(dag.fileloc, DagCode._get_code_from_file(dag.fileloc)).sync_to_db() url = 'code?dag_id=example_bash_operator' - resp = admin_client.get(url) + resp = admin_client.get(url, follow_redirects=True) check_content_not_in_response('Failed to load DAG file Code', resp) check_content_in_response('example_bash_operator', resp) @@ -358,7 +423,7 @@ def test_code_from_db_all_example_dags(admin_client): for dag in dagbag.dags.values(): DagCode(dag.fileloc, DagCode._get_code_from_file(dag.fileloc)).sync_to_db() url = 'code?dag_id=example_bash_operator' - resp = admin_client.get(url) + resp = admin_client.get(url, follow_redirects=True) check_content_not_in_response('Failed to load DAG file Code', resp) check_content_in_response('example_bash_operator', resp) From 3ae4b3c0eb0d92ecead3793d80fde222329d66b4 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Sun, 30 Jan 2022 20:36:19 +0100 Subject: [PATCH 2/6] Webserver - Change old path names to 'legacy_' --- airflow/www/decorators.py | 20 +++++-- airflow/www/templates/airflow/dag.html | 20 +++---- airflow/www/templates/airflow/dags.html | 10 ++-- airflow/www/utils.py | 4 +- airflow/www/views.py | 68 ++++++++++++++---------- tests/www/views/test_views_decorators.py | 20 ++++++- tests/www/views/test_views_tasks.py | 10 ++-- 7 files changed, 100 insertions(+), 52 deletions(-) diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 080fe682991c2..4aaeebde0c4cf 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -34,6 +34,15 @@ logger = logging.getLogger(__name__) +def _get_dag_id() -> str: + dag_id = request.args.get('dag_id') + if dag_id: + return dag_id + path_parts = request.path.split('/') + if len(path_parts) >= 3 and path_parts[1] == 'dags': + return path_parts[2] + + def action_logging(f: T) -> T: """Decorator to log user actions""" @@ -48,13 +57,18 @@ def wrapper(*args, **kwargs): user = g.user.username fields_skip_logging = {'csrf_token', '_csrf_token'} + log_fields = {k: v for k, v in request.values.items() if k not in fields_skip_logging} + dag_id = _get_dag_id() + if dag_id: + log_fields['dag_id'] = dag_id + log = Log( event=f.__name__, task_instance=None, owner=user, - extra=str([(k, v) for k, v in request.values.items() if k not in fields_skip_logging]), - task_id=request.values.get('task_id'), - dag_id=request.values.get('dag_id'), + extra=str([(k, log_fields[k]) for k in log_fields]), + task_id=log_fields.get('task_id'), + dag_id=log_fields.get('dag_id'), ) if 'execution_date' in request.values: diff --git a/airflow/www/templates/airflow/dag.html b/airflow/www/templates/airflow/dag.html index 594c4c1716e65..0f4b967373da5 100644 --- a/airflow/www/templates/airflow/dag.html +++ b/airflow/www/templates/airflow/dag.html @@ -110,33 +110,33 @@

@@ -182,7 +182,7 @@
- + Zoom into Sub DAG
@@ -402,7 +402,7 @@
more_horiz diff --git a/airflow/www/utils.py b/airflow/www/utils.py index cea50114e12b1..8cc420caccc5b 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -356,8 +356,10 @@ def dag_link(attr): """Generates a URL to the Graph view for a Dag.""" dag_id = attr.get('dag_id') execution_date = attr.get('execution_date') + if not dag_id: + return Markup('None') url = url_for('Airflow.graph', dag_id=dag_id, execution_date=execution_date) - return Markup('{}').format(url, dag_id) if dag_id else Markup('None') + return Markup('{}').format(url, dag_id) def dag_run_link(attr): diff --git a/airflow/www/views.py b/airflow/www/views.py index 2be6495a1c3bb..eb7ad8ebd6d34 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1054,9 +1054,9 @@ def last_dagruns(self, session=None): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), ] ) - def code(self): + def legacy_code(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_code', **request.args)) + return redirect(url_for('Airflow.code', **request.args)) @expose('/dags//code') @auth.has_access( @@ -1066,7 +1066,7 @@ def code(self): ] ) @provide_session - def dag_code(self, dag_id, session=None): + def code(self, dag_id, session=None): """Dag Code.""" all_errors = "" dag_orm = None @@ -1104,7 +1104,7 @@ def dag_code(self, dag_id, session=None): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), ] ) - def dag_details_redirect(self): + def legacy_dag_details(self): """Redirect from url param.""" return redirect(url_for('Airflow.dag_details', **request.args)) @@ -2332,7 +2332,21 @@ def success(self): @action_logging def dag(self, dag_id): """Redirect to default DAG view.""" - return redirect(url_for('Airflow.dag_grid', dag_id=dag_id, **request.args)) + return redirect(url_for('Airflow.grid', dag_id=dag_id, **request.args)) + + @expose('/legacy_tree') + @auth.has_access( + [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG), + ] + ) + @gzipped + @action_logging + def legacy_tree(self): + """Redirect to the replacement - grid view.""" + return redirect(url_for('Airflow.grid', **request.args)) @expose('/tree') @auth.has_access( @@ -2345,8 +2359,8 @@ def dag(self, dag_id): @gzipped @action_logging def tree(self): - """Redirect to the replacement - grid view.""" - return redirect(url_for('Airflow.dag_grid', **request.args)) + """Redirect to the replacement - grid view. Kept for backwards compatibility.""" + return redirect(url_for('Airflow.grid', **request.args)) @expose('/dags//grid') @auth.has_access( @@ -2359,8 +2373,8 @@ def tree(self): @gzipped @action_logging @provide_session - def dag_grid(self, dag_id, session=None): - """Get Dag as tree (grid).""" + def grid(self, dag_id, session=None): + """Get Dag's grid view.""" dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) if not dag: @@ -2445,9 +2459,9 @@ def dag_grid(self, dag_id, session=None): ) @gzipped @action_logging - def calendar(self): + def legacy_calendar(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_calendar', **request.args)) + return redirect(url_for('Airflow.calendar', **request.args)) @expose('/dags//calendar') @auth.has_access( @@ -2459,7 +2473,7 @@ def calendar(self): @gzipped @action_logging @provide_session - def dag_calendar(self, dag_id, session=None): + def calendar(self, dag_id, session=None): """Get DAG runs as calendar""" def _convert_to_date(session, column): @@ -2534,9 +2548,9 @@ def _convert_to_date(session, column): ) @gzipped @action_logging - def graph(self): + def legacy_graph(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_graph', **request.args)) + return redirect(url_for('Airflow.graph', **request.args)) @expose('/dags//graph') @auth.has_access( @@ -2549,7 +2563,7 @@ def graph(self): @gzipped @action_logging @provide_session - def dag_graph(self, dag_id, session=None): + def graph(self, dag_id, session=None): """Get DAG as Graph.""" dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) @@ -2640,9 +2654,9 @@ class GraphForm(DateTimeWithNumRunsWithDagRunsForm): ] ) @action_logging - def duration(self): + def legacy_duration(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_duration', **request.args)) + return redirect(url_for('Airflow.duration', **request.args)) @expose('/dags//duration') @auth.has_access( @@ -2653,7 +2667,7 @@ def duration(self): ) @action_logging @provide_session - def dag_duration(self, dag_id, session=None): + def duration(self, dag_id, session=None): """Get Dag as duration graph.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') dag_model = DagModel.get_dagmodel(dag_id) @@ -2793,9 +2807,9 @@ def dag_duration(self, dag_id, session=None): ] ) @action_logging - def tries(self): + def legacy_tries(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_tries', **request.args)) + return redirect(url_for('Airflow.tries', **request.args)) @expose('/dags//tries') @auth.has_access( @@ -2806,7 +2820,7 @@ def tries(self): ) @action_logging @provide_session - def dag_tries(self, dag_id, session=None): + def tries(self, dag_id, session=None): """Shows all tries.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') dag = current_app.dag_bag.get_dag(dag_id) @@ -2881,9 +2895,9 @@ def dag_tries(self, dag_id, session=None): ] ) @action_logging - def landing_times(self): + def legacy_landing_times(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_landing_times', **request.args)) + return redirect(url_for('Airflow.landing_times', **request.args)) @expose('/dags//landing_times') @auth.has_access( @@ -2894,7 +2908,7 @@ def landing_times(self): ) @action_logging @provide_session - def dag_landing_times(self, dag_id, session=None): + def landing_times(self, dag_id, session=None): """Shows landing times.""" default_dag_run = conf.getint('webserver', 'default_dag_run_display_number') dag: DAG = current_app.dag_bag.get_dag(dag_id) @@ -2997,9 +3011,9 @@ def paused(self): ] ) @action_logging - def gantt(self): + def legacy_gantt(self): """Redirect from url param.""" - return redirect(url_for('Airflow.dag_gantt', **request.args)) + return redirect(url_for('Airflow.gantt', **request.args)) @expose('/dags//gantt') @auth.has_access( @@ -3010,7 +3024,7 @@ def gantt(self): ) @action_logging @provide_session - def dag_gantt(self, dag_id, session=None): + def gantt(self, dag_id, session=None): """Show GANTT chart.""" dag = current_app.dag_bag.get_dag(dag_id) dag_model = DagModel.get_dagmodel(dag_id) diff --git a/tests/www/views/test_views_decorators.py b/tests/www/views/test_views_decorators.py index 9b5383f2d654e..c211806bd1b04 100644 --- a/tests/www/views/test_views_decorators.py +++ b/tests/www/views/test_views_decorators.py @@ -115,7 +115,7 @@ def _check_last_log(session, dag_id, event, execution_date): def test_action_logging_get(session, admin_client): url = ( - f'graph?dag_id=example_bash_operator&' + f'dags/example_bash_operator/graph?' f'execution_date={urllib.parse.quote_plus(str(EXAMPLE_DAG_DEFAULT_DATE))}' ) resp = admin_client.get(url, follow_redirects=True) @@ -131,6 +131,24 @@ def test_action_logging_get(session, admin_client): ) +def test_action_logging_get_legacy_view(session, admin_client): + url = ( + f'graph?dag_id=example_bash_operator&' + f'execution_date={urllib.parse.quote_plus(str(EXAMPLE_DAG_DEFAULT_DATE))}' + ) + resp = admin_client.get(url, follow_redirects=True) + check_content_in_response('runme_1', resp) + + # In mysql backend, this commit() is needed to write down the logs + session.commit() + _check_last_log( + session, + dag_id="example_bash_operator", + event="legacy_graph", + execution_date=EXAMPLE_DAG_DEFAULT_DATE, + ) + + def test_action_logging_post(session, admin_client): form = dict( task_id="runme_1", diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py index 3dc749fbbc852..c730e3e3f9c28 100644 --- a/tests/www/views/test_views_tasks.py +++ b/tests/www/views/test_views_tasks.py @@ -339,7 +339,7 @@ def test_tree_trigger_origin_tree_view(app, admin_client): url = 'tree?dag_id=test_tree_view' resp = admin_client.get(url, follow_redirects=True) - params = {'dag_id': 'test_tree_view', 'origin': '/tree?dag_id=test_tree_view'} + params = {'dag_id': 'test_tree_view', 'origin': '/dags/test_tree_view/grid'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) @@ -353,9 +353,9 @@ def test_graph_trigger_origin_graph_view(app, admin_client): state=State.RUNNING, ) - url = 'graph?dag_id=test_tree_view' + url = '/dags/test_tree_view/graph' resp = admin_client.get(url, follow_redirects=True) - params = {'dag_id': 'test_tree_view', 'origin': '/graph?dag_id=test_tree_view'} + params = {'dag_id': 'test_tree_view', 'origin': '/dags/test_tree_view/graph'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) @@ -369,9 +369,9 @@ def test_dag_details_trigger_origin_dag_details_view(app, admin_client): state=State.RUNNING, ) - url = 'dag_details?dag_id=test_graph_view' + url = '/dags/test_graph_view/details' resp = admin_client.get(url, follow_redirects=True) - params = {'dag_id': 'test_graph_view', 'origin': '/dag_details?dag_id=test_graph_view'} + params = {'dag_id': 'test_graph_view', 'origin': '/dags/test_graph_view/details'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) From a2a65bf8c7952b2ae03b14251d23fcac2f97a223 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Mon, 31 Jan 2022 22:29:41 +0100 Subject: [PATCH 3/6] Webserver - Use 'view_args' as optional source of 'dag_id' --- airflow/www/decorators.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 4aaeebde0c4cf..a12b9fb61f10d 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -20,7 +20,8 @@ import gzip import logging from io import BytesIO as IO -from typing import Callable, TypeVar, cast +from itertools import chain +from typing import Callable, Optional, TypeVar, cast import pendulum from flask import after_this_request, g, request @@ -34,15 +35,6 @@ logger = logging.getLogger(__name__) -def _get_dag_id() -> str: - dag_id = request.args.get('dag_id') - if dag_id: - return dag_id - path_parts = request.path.split('/') - if len(path_parts) >= 3 and path_parts[1] == 'dags': - return path_parts[2] - - def action_logging(f: T) -> T: """Decorator to log user actions""" @@ -57,10 +49,10 @@ def wrapper(*args, **kwargs): user = g.user.username fields_skip_logging = {'csrf_token', '_csrf_token'} - log_fields = {k: v for k, v in request.values.items() if k not in fields_skip_logging} - dag_id = _get_dag_id() - if dag_id: - log_fields['dag_id'] = dag_id + log_fields = {k: v for k, v in chain( + request.values.items(), + request.view_args.items() + ) if k not in fields_skip_logging} log = Log( event=f.__name__, From ba01d65e15b8873f8ccc1c99da3e167043849591 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Mon, 31 Jan 2022 22:45:34 +0100 Subject: [PATCH 4/6] Webserver - Do not use 'legacy_tree' view in the link meta --- airflow/www/templates/airflow/dags.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/www/templates/airflow/dags.html b/airflow/www/templates/airflow/dags.html index 639f525beca3e..41f63bf6b8ad1 100644 --- a/airflow/www/templates/airflow/dags.html +++ b/airflow/www/templates/airflow/dags.html @@ -39,7 +39,7 @@ - + {% endblock %} {% block head_css %} From cb0c8277b7d4ff672563670d0abb44306dd22aea Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Tue, 1 Feb 2022 22:52:55 +0100 Subject: [PATCH 5/6] Webserver - Fixing static checks --- airflow/www/decorators.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index a12b9fb61f10d..47a9847c2eecf 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -21,7 +21,7 @@ import logging from io import BytesIO as IO from itertools import chain -from typing import Callable, Optional, TypeVar, cast +from typing import Callable, TypeVar, cast import pendulum from flask import after_this_request, g, request @@ -49,10 +49,11 @@ def wrapper(*args, **kwargs): user = g.user.username fields_skip_logging = {'csrf_token', '_csrf_token'} - log_fields = {k: v for k, v in chain( - request.values.items(), - request.view_args.items() - ) if k not in fields_skip_logging} + log_fields = { + k: v + for k, v in chain(request.values.items(), request.view_args.items()) + if k not in fields_skip_logging + } log = Log( event=f.__name__, From 875256ed8e4930c85de87708df42670c9ba2f0a8 Mon Sep 17 00:00:00 2001 From: Igor Kholopov Date: Tue, 1 Feb 2022 23:03:23 +0100 Subject: [PATCH 6/6] Webserver - Use hyphen instead of underscore in URL --- airflow/www/views.py | 2 +- tests/www/views/test_views_tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index eb7ad8ebd6d34..b255ec67e8ff0 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -2899,7 +2899,7 @@ def legacy_landing_times(self): """Redirect from url param.""" return redirect(url_for('Airflow.landing_times', **request.args)) - @expose('/dags//landing_times') + @expose('/dags//landing-times') @auth.has_access( [ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py index c730e3e3f9c28..bd0c7badd46f2 100644 --- a/tests/www/views/test_views_tasks.py +++ b/tests/www/views/test_views_tasks.py @@ -217,7 +217,7 @@ def client_ti_without_dag_edit(app): id='landing-times-url-param', ), pytest.param( - 'dags/example_bash_operator/landing_times?days=30', + 'dags/example_bash_operator/landing-times?days=30', ['example_bash_operator'], id='landing-times', ),