This is an automated email from the ASF dual-hosted git repository. ash pushed a commit to branch revert-20730-ikholopov/url_single_path_dag_page in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 2dc9233fe3b249976e37e2bf7ffcb5c9676c8fd8 Author: Ash Berlin-Taylor <[email protected]> AuthorDate: Thu Feb 10 10:46:56 2022 +0000 Revert "Modernize DAG-related URL routes and rename "tree" to "grid" (#20730)" This reverts commit f217becdfc371ea18486886cc3b2f47eeda0f77f. --- airflow/www/decorators.py | 13 +-- airflow/www/templates/airflow/dag.html | 10 +- airflow/www/templates/airflow/dags.html | 8 +- airflow/www/utils.py | 4 +- airflow/www/views.py | 169 ++++--------------------------- tests/www/views/test_views_decorators.py | 20 +--- tests/www/views/test_views_tasks.py | 85 ++-------------- 7 files changed, 44 insertions(+), 265 deletions(-) diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 47a9847..080fe68 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -20,7 +20,6 @@ import functools import gzip import logging from io import BytesIO as IO -from itertools import chain from typing import Callable, TypeVar, cast import pendulum @@ -49,19 +48,13 @@ def action_logging(f: T) -> T: 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 = Log( event=f.__name__, task_instance=None, owner=user, - 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'), + 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'), ) if 'execution_date' in request.values: diff --git a/airflow/www/templates/airflow/dag.html b/airflow/www/templates/airflow/dag.html index 0f4b967..e57a1f1 100644 --- a/airflow/www/templates/airflow/dag.html +++ b/airflow/www/templates/airflow/dag.html @@ -110,9 +110,9 @@ <div class="row"> <div class="col-md-10"> <ul class="nav nav-pills"> - <li><a href="{{ url_for('Airflow.grid', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}"> - <span class="material-icons" aria-hidden="true">grid_on</span> - Grid + <li><a href="{{ url_for('Airflow.tree', dag_id=dag.dag_id, num_runs=num_runs_arg, root=root, base_date=base_date_arg) }}"> + <span class="material-icons" aria-hidden="true">nature</span> + Tree </a></li> <li><a href="{{ url_for('Airflow.graph', dag_id=dag.dag_id, root=root, num_runs=num_runs_arg, base_date=base_date_arg, execution_date=execution_date_arg) }}"> <span class="material-icons" aria-hidden="true">account_tree</span> @@ -182,7 +182,7 @@ </div> <div class="modal-body"> <div id="div_btn_subdag"> - <a id="btn_subdag" class="btn btn-primary" data-base-url="{{ url_for('Airflow.legacy_' + dag.default_view) }}"> + <a id="btn_subdag" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.default_view) }}"> Zoom into Sub DAG </a> <hr> @@ -402,7 +402,7 @@ </form> </span> <span class="col-md-4 text-right"> - <a id="btn_dag_graph_view" class="btn" data-base-url="{{ url_for('Airflow.graph', dag_id=dag.dag_id) }}" role="button"> + <a id="btn_dag_graph_view" class="btn" data-base-url="{{ url_for('Airflow.graph') }}" role="button"> <span class="material-icons" aria-hidden="true">account_tree</span> Graph </a> diff --git a/airflow/www/templates/airflow/dags.html b/airflow/www/templates/airflow/dags.html index 41f63bf..c748d32 100644 --- a/airflow/www/templates/airflow/dags.html +++ b/airflow/www/templates/airflow/dags.html @@ -31,7 +31,7 @@ <meta name="paused_url" content="{{ url_for('Airflow.paused') }}"> <meta name="status_filter" content="{{ status_filter }}"> <meta name="autocomplete_url" content="{{ url_for('AutocompleteView.autocomplete') }}"> - <meta name="graph_url" content="{{ url_for('Airflow.legacy_graph') }}"> + <meta name="graph_url" content="{{ url_for('Airflow.graph') }}"> <meta name="dag_run_url" content="{{ url_for('DagRunModelView.list') }}"> <meta name="task_instance_url" content="{{ url_for('TaskInstanceModelView.list') }}"> <meta name="blocked_url" content="{{ url_for('Airflow.blocked') }}"> @@ -293,9 +293,9 @@ <span class="material-icons" aria-hidden="true">account_tree</span> Graph </a> - <a href="{{ url_for('Airflow.grid', dag_id=dag.dag_id, num_runs=num_runs) }}" class="dags-table-more__link"> - <span class="material-icons" aria-hidden="true">grid_on</span> - Grid + <a href="{{ url_for('Airflow.tree', dag_id=dag.dag_id, num_runs=num_runs) }}" class="dags-table-more__link"> + <span class="material-icons" aria-hidden="true">nature</span> + Tree </a> </div> <span class="dags-table-more__toggle"><span class="material-icons">more_horiz</span></span> diff --git a/airflow/www/utils.py b/airflow/www/utils.py index d9ec10f..18eb1e7 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -356,10 +356,8 @@ 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('<a href="{}">{}</a>').format(url, dag_id) + return Markup('<a href="{}">{}</a>').format(url, dag_id) if dag_id else Markup('None') def dag_run_link(attr): diff --git a/airflow/www/views.py b/airflow/www/views.py index 7ff2c74..678d1cc 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1053,24 +1053,15 @@ class Airflow(AirflowBaseView): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), ] ) - def legacy_code(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.code', **request.args)) - - @expose('/dags/<string:dag_id>/code') - @auth.has_access( - [ - (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), - (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), - ] - ) @provide_session - def code(self, dag_id, session=None): + def code(self, 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))) @@ -1103,20 +1094,10 @@ class Airflow(AirflowBaseView): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), ] ) - def legacy_dag_details(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.dag_details', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def dag_details(self, 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) @@ -2319,34 +2300,6 @@ class Airflow(AirflowBaseView): State.SUCCESS, ) - @expose('/dags/<string:dag_id>') - @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.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( [ @@ -2357,23 +2310,10 @@ class Airflow(AirflowBaseView): ) @gzipped @action_logging - def tree(self): - """Redirect to the replacement - grid view. Kept for backwards compatibility.""" - return redirect(url_for('Airflow.grid', **request.args)) - - @expose('/dags/<string:dag_id>/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 grid(self, dag_id, session=None): - """Get Dag's grid view.""" + def tree(self, session=None): + """Get Dag as tree.""" + 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: @@ -2458,21 +2398,8 @@ class Airflow(AirflowBaseView): ) @gzipped @action_logging - def legacy_calendar(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.calendar', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def calendar(self, session=None): """Get DAG runs as calendar""" def _convert_to_date(session, column): @@ -2482,6 +2409,7 @@ class Airflow(AirflowBaseView): 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: @@ -2547,23 +2475,10 @@ class Airflow(AirflowBaseView): ) @gzipped @action_logging - def legacy_graph(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.graph', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def graph(self, 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: @@ -2653,22 +2568,11 @@ class Airflow(AirflowBaseView): ] ) @action_logging - def legacy_duration(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.duration', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def duration(self, 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) @@ -2806,22 +2710,11 @@ class Airflow(AirflowBaseView): ] ) @action_logging - def legacy_tries(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.tries', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def tries(self, 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') @@ -2894,22 +2787,11 @@ class Airflow(AirflowBaseView): ] ) @action_logging - def legacy_landing_times(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.landing_times', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def landing_times(self, 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') @@ -3010,21 +2892,10 @@ class Airflow(AirflowBaseView): ] ) @action_logging - def legacy_gantt(self): - """Redirect from url param.""" - return redirect(url_for('Airflow.gantt', **request.args)) - - @expose('/dags/<string:dag_id>/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, dag_id, session=None): + def gantt(self, 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_decorators.py b/tests/www/views/test_views_decorators.py index c211806..9b5383f 100644 --- a/tests/www/views/test_views_decorators.py +++ b/tests/www/views/test_views_decorators.py @@ -115,24 +115,6 @@ def _check_last_log(session, dag_id, event, execution_date): def test_action_logging_get(session, admin_client): url = ( - 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) - 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="graph", - execution_date=EXAMPLE_DAG_DEFAULT_DATE, - ) - - -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))}' ) @@ -144,7 +126,7 @@ def test_action_logging_get_legacy_view(session, admin_client): _check_last_log( session, dag_id="example_bash_operator", - event="legacy_graph", + event="graph", execution_date=EXAMPLE_DAG_DEFAULT_DATE, ) diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py index bd0c7ba..d433542 100644 --- a/tests/www/views/test_views_tasks.py +++ b/tests/www/views/test_views_tasks.py @@ -139,26 +139,16 @@ def client_ti_without_dag_edit(app): pytest.param( 'dag_details?dag_id=example_bash_operator', ['DAG Details'], - id="dag-details-url-param", + id="dag-details", ), 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( @@ -167,68 +157,33 @@ def client_ti_without_dag_edit(app): 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-url-param", - ), - pytest.param( - 'dags/example_subdag_operator.section-1/grid', - ['section-1-task-1'], - id="grid-subdag", + id="tree-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( @@ -241,41 +196,21 @@ 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-url-param", - ), - pytest.param( - "dags/example_bash_operator/grid", - ["example_bash_operator"], - id="existing-dagbag-grid", + id="existing-dagbag-tree", ), 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( @@ -339,7 +274,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': '/dags/test_tree_view/grid'} + params = {'dag_id': 'test_tree_view', 'origin': '/tree?dag_id=test_tree_view'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) @@ -353,9 +288,9 @@ def test_graph_trigger_origin_graph_view(app, admin_client): state=State.RUNNING, ) - url = '/dags/test_tree_view/graph' + url = 'graph?dag_id=test_tree_view' resp = admin_client.get(url, follow_redirects=True) - params = {'dag_id': 'test_tree_view', 'origin': '/dags/test_tree_view/graph'} + params = {'dag_id': 'test_tree_view', 'origin': '/graph?dag_id=test_tree_view'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) @@ -369,9 +304,9 @@ def test_dag_details_trigger_origin_dag_details_view(app, admin_client): state=State.RUNNING, ) - url = '/dags/test_graph_view/details' + url = 'dag_details?dag_id=test_graph_view' resp = admin_client.get(url, follow_redirects=True) - params = {'dag_id': 'test_graph_view', 'origin': '/dags/test_graph_view/details'} + params = {'dag_id': 'test_graph_view', 'origin': '/dag_details?dag_id=test_graph_view'} href = f"/trigger?{html.escape(urllib.parse.urlencode(params))}" check_content_in_response(href, resp) @@ -413,7 +348,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, follow_redirects=True) + resp = admin_client.get(url) check_content_not_in_response('Failed to load DAG file Code', resp) check_content_in_response('example_bash_operator', resp) @@ -423,7 +358,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, follow_redirects=True) + resp = admin_client.get(url) check_content_not_in_response('Failed to load DAG file Code', resp) check_content_in_response('example_bash_operator', resp)
