You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by je...@apache.org on 2021/07/29 17:09:43 UTC

[airflow] branch main updated: Remove DAG refresh buttons (#17263)

This is an automated email from the ASF dual-hosted git repository.

jedcunningham 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 1394dbe  Remove DAG refresh buttons (#17263)
1394dbe is described below

commit 1394dbe4001fd451896c334a8a6011c6679d5345
Author: Jed Cunningham <66...@users.noreply.github.com>
AuthorDate: Thu Jul 29 11:09:28 2021 -0600

    Remove DAG refresh buttons (#17263)
    
    Now that the DAG parser syncs DAG specific permissions there really
    isn't a need to manually refresh DAGs via the UI.
---
 UPDATING.md                                     |  6 ++++
 airflow/www/templates/airflow/dag.html          |  3 --
 airflow/www/templates/airflow/dags.html         |  3 --
 airflow/www/views.py                            | 42 -------------------------
 docs/apache-airflow/security/access-control.rst |  2 --
 tests/www/views/test_views_acl.py               | 24 ++------------
 tests/www/views/test_views_tasks.py             | 12 -------
 7 files changed, 9 insertions(+), 83 deletions(-)

diff --git a/UPDATING.md b/UPDATING.md
index 4238652..4777b0e 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -157,6 +157,12 @@ not have any effect in an existing deployment where the ``default_pool`` already
 
 Previously this was controlled by `non_pooled_task_slot_count` in `[core]` section, which was not documented.
 
+### Webserver DAG refresh buttons removed
+
+Now that the DAG parser syncs DAG permissions there is no longer a need for manually refreshing DAGs. As such, the buttons to refresh a DAG have been removed from the UI.
+
+In addition, the `/refresh` and `/refresh_all` webserver endpoints have also been removed.
+
 ## Airflow 2.1.1
 
 ### `activate_dag_runs` argument of the function `clear_task_instances` is replaced with `dag_run_state`
diff --git a/airflow/www/templates/airflow/dag.html b/airflow/www/templates/airflow/dag.html
index 322d19f..2c4c25c 100644
--- a/airflow/www/templates/airflow/dag.html
+++ b/airflow/www/templates/airflow/dag.html
@@ -144,9 +144,6 @@
               <li><a href="{{ url_for('Airflow.trigger', dag_id=dag.dag_id, origin=url_for('Airflow.' + dag.default_view, dag_id=dag.dag_id)) }}">Trigger DAG w/ config</a></li>
             </ul>
           </div>
-          <a href="{{ url_for('Airflow.refresh', dag_id=dag.dag_id) }}" title="Refresh DAG" aria-label="Refresh DAG" onclick="postAsForm(this.href); return false" class="btn btn-default btn-icon-only{{ ' disabled' if not dag.can_edit }}">
-            <span class="material-icons" aria-hidden="true">refresh</span>
-          </a>
           <a href="{{ url_for('Airflow.delete', dag_id=dag.dag_id) }}" title="Delete&nbsp;DAG" class="btn btn-default btn-icon-only{{ ' disabled' if not dag.can_delete }}"
             onclick="return confirmDeleteDag(this, '{{ dag.safe_dag_id }}')" aria-label="Delete DAG">
             <span class="material-icons text-danger" aria-hidden="true">delete_outline</span>
diff --git a/airflow/www/templates/airflow/dags.html b/airflow/www/templates/airflow/dags.html
index 290675c..bdc1190 100644
--- a/airflow/www/templates/airflow/dags.html
+++ b/airflow/www/templates/airflow/dags.html
@@ -195,9 +195,6 @@
                         <li><a href="{{ url_for('Airflow.trigger', dag_id=dag.dag_id) }}">Trigger DAG w/ config</a></li>
                       </ul>
                     </div>
-                      <a href="{{ url_for('Airflow.refresh', dag_id=dag.dag_id) }}" onclick="postAsForm(this.href); return false" title="Refresh DAG" aria-label="Refresh DAG" class="btn btn-sm btn-default btn-icon-only {{ ' disabled' if not dag.can_edit }}">
-                        <span class="material-icons" aria-hidden="true">refresh</span>
-                      </a>
                     {% endif %}
                     {# Use dag_id instead of dag.dag_id, because the DAG might not exist in the webserver's DagBag #}
                     <a href="{{ url_for('Airflow.delete', dag_id=dag.dag_id) }}" onclick="return confirmDeleteDag(this, '{{ dag.dag_id }}')" title="Delete&nbsp;DAG" aria-label="Delete DAG" class="btn btn-sm btn-default btn-icon-only {{ ' disabled' if not dag.can_delete }}">
diff --git a/airflow/www/views.py b/airflow/www/views.py
index 117b173..8309d99 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -2617,48 +2617,6 @@ class Airflow(AirflowBaseView):
         models.DagModel.get_dagmodel(dag_id).set_is_paused(is_paused=is_paused)
         return "OK"
 
-    @expose('/refresh', methods=['POST'])
-    @auth.has_access(
-        [
-            (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),
-        ]
-    )
-    @action_logging
-    @provide_session
-    def refresh(self, session=None):
-        """Refresh DAG."""
-        dag_id = request.values.get('dag_id')
-        orm_dag = session.query(DagModel).filter(DagModel.dag_id == dag_id).first()
-
-        if orm_dag:
-            orm_dag.last_expired = timezone.utcnow()
-            session.merge(orm_dag)
-        session.commit()
-
-        dag = current_app.dag_bag.get_dag(dag_id)
-        # sync dag permission
-        current_app.appbuilder.sm.sync_perm_for_dag(dag_id, dag.access_control)
-
-        flash(f"DAG [{dag_id}] is now fresh as a daisy")
-        return redirect(request.referrer)
-
-    @expose('/refresh_all', methods=['POST'])
-    @auth.has_access(
-        [
-            (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG),
-        ]
-    )
-    @action_logging
-    def refresh_all(self):
-        """Refresh everything"""
-        current_app.dag_bag.collect_dags_from_db()
-
-        # sync permissions for all dags
-        for dag_id, dag in current_app.dag_bag.dags.items():
-            current_app.appbuilder.sm.sync_perm_for_dag(dag_id, dag.access_control)
-        flash("All DAGs are now up to date")
-        return redirect(url_for('Airflow.index'))
-
     @expose('/gantt')
     @auth.has_access(
         [
diff --git a/docs/apache-airflow/security/access-control.rst b/docs/apache-airflow/security/access-control.rst
index 2615725..0840435 100644
--- a/docs/apache-airflow/security/access-control.rst
+++ b/docs/apache-airflow/security/access-control.rst
@@ -223,8 +223,6 @@ Get DAG as duration graph              DAGs.can_read, Task Instances.can_read
 Show all tries                         DAGs.can_read, Task Instances.can_read                                  Viewer
 Show landing times                     DAGs.can_read, Task Instances.can_read                                  Viewer
 Toggle DAG paused status               DAGs.can_edit                                                           User
-Refresh DAG                            DAGs.can_edit                                                           User
-Refresh all DAGs                       DAGs.can_edit                                                           User
 Show Gantt Chart                       DAGs.can_read, Task Instances.can_read                                  Viewer
 Get external links                     DAGs.can_read, Task Instances.can_read                                  Viewer
 Show Task Instances                    DAGs.can_read, Task Instances.can_read                                  Viewer
diff --git a/tests/www/views/test_views_acl.py b/tests/www/views/test_views_acl.py
index 8fe011b..4dab386 100644
--- a/tests/www/views/test_views_acl.py
+++ b/tests/www/views/test_views_acl.py
@@ -694,21 +694,9 @@ def test_failed_success(client_all_dags_edit_tis):
     check_content_in_response('Marked failed on 1 task instances', resp)
 
 
-@pytest.mark.parametrize(
-    "url, expected_content",
-    [
-        ("paused?dag_id=example_bash_operator&is_paused=false", "OK"),
-        ("refresh?dag_id=example_bash_operator", ""),
-    ],
-    ids=[
-        "paused",
-        "refresh",
-    ],
-)
-def test_post_success(dag_test_client, url, expected_content):
-    # post request failure won't test
-    resp = dag_test_client.post(url, follow_redirects=True)
-    check_content_in_response(expected_content, resp)
+def test_paused_post_success(dag_test_client):
+    resp = dag_test_client.post("paused?dag_id=example_bash_operator&is_paused=false", follow_redirects=True)
+    check_content_in_response("OK", resp)
 
 
 @pytest.fixture(scope="module")
@@ -771,9 +759,3 @@ def test_get_logs_with_metadata_failure(dag_faker_client):
     )
     check_content_not_in_response('"message":', resp)
     check_content_not_in_response('"metadata":', resp)
-
-
-def test_refresh_failure_for_viewer(viewer_client):
-    # viewer role can't refresh
-    resp = viewer_client.post('refresh?dag_id=example_bash_operator')
-    check_content_in_response('Redirecting', resp, resp_code=302)
diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py
index cf9b95b..04db6fc 100644
--- a/tests/www/views/test_views_tasks.py
+++ b/tests/www/views/test_views_tasks.py
@@ -502,18 +502,6 @@ def test_run_with_not_runnable_states(_, admin_client, session, state):
     assert re.search(msg, resp.get_data(as_text=True))
 
 
-def test_refresh(admin_client):
-    resp = admin_client.post('refresh?dag_id=example_bash_operator')
-    check_content_in_response('', resp, resp_code=302)
-
-
-def test_refresh_all(app, admin_client):
-    with unittest.mock.patch.object(app.dag_bag, 'collect_dags_from_db') as collect_dags_from_db:
-        resp = admin_client.post("/refresh_all", follow_redirects=True)
-        check_content_in_response('', resp)
-        collect_dags_from_db.assert_called_once_with()
-
-
 @pytest.fixture()
 def new_id_example_bash_operator():
     dag_id = 'example_bash_operator'