You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by as...@apache.org on 2021/09/24 11:53:10 UTC
[airflow] branch main updated: Don't use flash for "same-page" UI
messages. (#18462)
This is an automated email from the ASF dual-hosted git repository.
ash pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 18e91bc Don't use flash for "same-page" UI messages. (#18462)
18e91bc is described below
commit 18e91bcde0922ded6eed724924635a31578d8230
Author: Ash Berlin-Taylor <as...@firemirror.com>
AuthorDate: Fri Sep 24 12:52:54 2021 +0100
Don't use flash for "same-page" UI messages. (#18462)
The flash is designed for setting a message on one page and then showing
it after a redirect, so in the case of these UI warnings they were being
shown twice due to an early redirect.
I could have chosen to fix this by moving the checks to after the
`reset_tags` redirect, but it _also_ felt wrong to me to have HTML in
the view, so I have chosen to move it in to the template where it
belongs.
To (marginally) reduce boilerplate I have created `message()` "macro"
(Jinja macro, a.k.a. function; not to be confused with what Airflow
templates call a macro, but is in fact just a template global) that
handles the formatting for messages.
---
airflow/www/templates/airflow/_messages.html | 30 ++++++++++++++++++++
airflow/www/templates/airflow/dags.html | 21 ++++++++++++++
airflow/www/templates/airflow/main.html | 9 +++---
airflow/www/templates/appbuilder/flash.html | 19 +++----------
airflow/www/views.py | 41 ++++++++--------------------
tests/www/views/test_views.py | 3 ++
6 files changed, 74 insertions(+), 49 deletions(-)
diff --git a/airflow/www/templates/airflow/_messages.html b/airflow/www/templates/airflow/_messages.html
new file mode 100644
index 0000000..56fa505
--- /dev/null
+++ b/airflow/www/templates/airflow/_messages.html
@@ -0,0 +1,30 @@
+{#
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+#}
+{%- macro message(content, category='info', dismissable=true) -%}
+ <div class="alert alert-{{ category }}">
+ {%- if dismissable -%}
+ <button type="button" class="close" data-dismiss="alert">×</button>
+ {%- endif -%}
+ {%- if caller is defined -%}
+ {{ caller() }}
+ {%- else -%}
+ {{ content }}
+ {%- endif -%}
+ </div>
+{%- endmacro -%}
diff --git a/airflow/www/templates/airflow/dags.html b/airflow/www/templates/airflow/dags.html
index 0db1feb..cf3b4c0 100644
--- a/airflow/www/templates/airflow/dags.html
+++ b/airflow/www/templates/airflow/dags.html
@@ -19,6 +19,7 @@
{% extends base_template %}
{% from 'appbuilder/loading_dots.html' import loading_dots %}
+{% from 'airflow/_messages.html' import message %}
{% block page_title %}
{% if search_query %}"{{ search_query }}" - {% endif %}DAGs - {{ appbuilder.app_name }}
@@ -46,6 +47,26 @@
<link rel="stylesheet" type="text/css" href="{{ url_for_asset('dags.css') }}">
{% endblock %}
+{% block messages %}
+ {% for m in dashboard_alerts %}
+ {{ message(m.message, m.category) }}
+ {% endfor %}
+ {{ super() }}
+ {% if sqlite_warning | default(true) %}
+ {% call message(category='warning', dismissable=false) %}
+ Do not use <b>SQLite</b> as metadata DB in production – it should only be used for dev/testing
+ We recommend using Postgres or MySQL.
+ <a href={{ get_docs_url("howto/set-up-database.html") }}><b>Click here</b></a> for more information.
+ {% endcall %}
+ {% endif %}
+ {% if sequential_executor_warning | default(false) %}
+ {% call message(category='warning', dismissable=false) %}
+ Do not use <b>SequentialExecutor</b> in production.
+ <a href={{ get_docs_url("executor/index.html") }}><b>Click here</b></a> for more information.
+ {% endcall %}
+ {% endif %}
+{% endblock %}
+
{% block content %}
<h2>{{ page_title }}</h2>
<div id="main_content">
diff --git a/airflow/www/templates/airflow/main.html b/airflow/www/templates/airflow/main.html
index 57a3d07..f9190b4 100644
--- a/airflow/www/templates/airflow/main.html
+++ b/airflow/www/templates/airflow/main.html
@@ -18,6 +18,7 @@
#}
{% extends 'appbuilder/baselayout.html' %}
+{% from 'airflow/_messages.html' import message %}
{% block page_title -%}
{% if title is defined -%}
@@ -51,7 +52,7 @@
{% block messages %}
{% include 'appbuilder/flash.html' %}
{% if scheduler_job is defined and (not scheduler_job or not scheduler_job.is_alive()) %}
- <div class="alert alert-warning">
+ {% call message(category='warning', dismissable=false) %}
<p>The scheduler does not appear to be running.
{% if scheduler_job %}
Last heartbeat was received
@@ -63,10 +64,10 @@
{% endif %}
</p>
<p>The DAGs list may not update, and new tasks will not be scheduled.</p>
- </div>
+ {% endcall %}
{% endif %}
{% if triggerer_job is defined and (not triggerer_job or not triggerer_job.is_alive()) %}
- <div class="alert alert-warning">
+ {% call message(category='warning', dismissable=false) %}
<p>The triggerer does not appear to be running.
{% if triggerer_job %}
Last heartbeat was received
@@ -78,7 +79,7 @@
{% endif %}
</p>
<p>Triggers will not run, and any deferred operator will remain deferred until it times out and fails.</p>
- </div>
+ {% endcall %}
{% endif %}
{% endblock %}
diff --git a/airflow/www/templates/appbuilder/flash.html b/airflow/www/templates/appbuilder/flash.html
index ea54046..0f97c9c 100644
--- a/airflow/www/templates/appbuilder/flash.html
+++ b/airflow/www/templates/appbuilder/flash.html
@@ -17,9 +17,9 @@
under the License.
#}
-{#
+{#-
Adapted from: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/templates/appbuilder/flash.html
-#}
+-#}
<link rel="stylesheet" type="text/css" href="{{ url_for_asset('flash.css') }}">
{#
@@ -33,20 +33,9 @@
{% for category, m in messages %}
{% if category == 'dag_import_error' %}
{{ dag_import_errors.append((category, m)) if dag_import_errors.append((category, m)) != None else '' }}
- {% else %}
- {{ regular_alerts.append((category, m)) if regular_alerts.append((category, m)) != None else '' }}
- {% endif %}
- {% endfor %}
- {% endif %}
-
- {% if regular_alerts %}
- {% for category, m in regular_alerts %}
- {% if not (request.path == appbuilder.get_url_for_login and 'access is denied' in m.lower()) %}
+ {% elif not (request.path == appbuilder.get_url_for_login and 'access is denied' in m.lower()) %}
{# Don't show 'Access is Denied' alert if user is logged out and on the login page. #}
- <div class="alert alert-{{ category if category else 'info' }}">
- <button type="button" class="close" data-dismiss="alert">×</button>
- {{ m }}
- </div>
+ {{ message(m, category) }}
{% endif %}
{% endfor %}
{% endif %}
diff --git a/airflow/www/views.py b/airflow/www/views.py
index 919dc19..115a423 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -456,11 +456,16 @@ class AirflowBaseView(BaseView):
route_base = ''
- # Make our macros available to our UI templates too.
extra_args = {
+ # Make our macros available to our UI templates too.
'macros': macros,
+ 'get_docs_url': get_docs_url,
}
+ if not conf.getboolean('core', 'unit_test_mode'):
+ extra_args['sqlite_warning'] = settings.Session.bind.dialect.name == 'sqlite'
+ extra_args['sequential_executor_warning'] = conf.get('core', 'executor') == 'LocalExecutor'
+
line_chart_attr = {
'legend.maxKeyLength': 200,
}
@@ -541,35 +546,6 @@ class Airflow(AirflowBaseView):
)
def index(self):
"""Home view."""
- unit_test_mode: bool = conf.getboolean('core', 'UNIT_TEST_MODE')
-
- if not unit_test_mode and "sqlite" in conf.get("core", "sql_alchemy_conn"):
- db_doc_page = get_docs_url("howto/set-up-database.html")
- flash(
- Markup(
- "Usage of <b>SQLite</b> detected. It should only be used for dev/testing. "
- "Do not use <b>SQLite</b> as metadata DB in production. "
- "We recommend using Postgres or MySQL. "
- f"<a href='{db_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
-
- if not unit_test_mode and conf.get("core", "executor") == "SequentialExecutor":
- exec_doc_page = get_docs_url("executor/index.html")
- flash(
- Markup(
- "Usage of <b>SequentialExecutor</b> detected. "
- "Do not use <b>SequentialExecutor</b> in production. "
- f"<a href='{exec_doc_page}'><b>Click here</b></a> for more information."
- ),
- category="warning",
- )
-
- for fm in settings.DASHBOARD_UIALERTS:
- if fm.should_show(current_app.appbuilder.sm):
- flash(fm.message, fm.category)
-
hide_paused_dags_by_default = conf.getboolean('webserver', 'hide_paused_dags_by_default')
default_dag_run = conf.getint('webserver', 'default_dag_run_display_number')
@@ -712,9 +688,14 @@ class Airflow(AirflowBaseView):
page_title = conf.get(section="webserver", key="instance_name", fallback="DAGs")
+ dashboard_alerts = [
+ fm for fm in settings.DASHBOARD_UIALERTS if fm.should_show(current_app.appbuilder.sm)
+ ]
+
return self.render_template(
'airflow/dags.html',
dags=dags,
+ dashboard_alerts=dashboard_alerts,
current_page=current_page,
search_query=arg_search_query if arg_search_query else '',
page_title=page_title,
diff --git a/tests/www/views/test_views.py b/tests/www/views/test_views.py
index f47e59f..b98c1bc 100644
--- a/tests/www/views/test_views.py
+++ b/tests/www/views/test_views.py
@@ -54,6 +54,8 @@ def test_configuration_expose_config(admin_client):
def test_redoc_should_render_template(capture_templates, admin_client):
+ from airflow.utils.docs import get_docs_url
+
with capture_templates() as templates:
resp = admin_client.get('redoc')
check_content_in_response('Redoc', resp)
@@ -63,6 +65,7 @@ def test_redoc_should_render_template(capture_templates, admin_client):
assert templates[0].local_context == {
'openapi_spec_url': '/api/v1/openapi.yaml',
'rest_api_enabled': True,
+ 'get_docs_url': get_docs_url,
}