You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2022/08/04 19:37:00 UTC

[airflow] 05/07: Limit Flask to <2.3 in the wake of 2.2 breaking our tests (#25511)

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

potiuk pushed a commit to branch v2-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit 71da6179781734cbebf4c0ec12f2bccd2b5aef5e
Author: Jarek Potiuk <ja...@polidea.com>
AuthorDate: Wed Aug 3 21:04:41 2022 +0200

    Limit Flask to <2.3 in the wake of 2.2 breaking our tests (#25511)
    
    Flask 2.2 added a few deprecations and made a few changes that
    made our tests stop working. Those were really test problems not
    real application problems (there were no breaking changes in 2.2):
    
    * new deprecation warnings produced
    * Flask app cannot be reused in multiple tests
    * The way how session lifetime is calculated makes test fail
      if freezegun is frozen using Pendulum datetime rather than the
      stdlib ones
    
    This is an early warning for the future as the deprecation
    warnings make us aware that Flask 2.3 is breaking. So this PR
    fixes the 2.2 compatibility but at the same time limits Flask to
    < 2.3 with appropriate information when the limit can be removed.
    
    (cherry picked from commit 42ea000401c50d8b47e791b746859854420ac0f6)
---
 scripts/in_container/verify_providers.py        | 31 +++++++++----------
 setup.cfg                                       |  5 +++-
 tests/www/views/test_views_custom_user_views.py | 20 +++++++------
 tests/www/views/test_views_grid.py              | 40 +++++++++++++++----------
 tests/www/views/test_views_tasks.py             | 13 ++++----
 5 files changed, 60 insertions(+), 49 deletions(-)

diff --git a/scripts/in_container/verify_providers.py b/scripts/in_container/verify_providers.py
index 46586d7b5b..b85df02fcf 100755
--- a/scripts/in_container/verify_providers.py
+++ b/scripts/in_container/verify_providers.py
@@ -187,6 +187,14 @@ KNOWN_DEPRECATED_MESSAGES: Set[Tuple[str, str]] = {
         "going out for this old package name.",
         "scrapbook",
     ),
+    ("SelectableGroups dict interface is deprecated. Use select.", "markdown"),
+    ("'_app_ctx_stack' is deprecated and will be removed in Flask 2.3.", "flask_sqlalchemy"),
+    ("'_app_ctx_stack' is deprecated and will be removed in Flask 2.3.", "flask_appbuilder"),
+    # Currently (2.2) Flask app builder has the `remoevd` typo in the messages,
+    # and they might want to fix it, so adding both
+    ("'_request_ctx_stack' is deprecated and will be remoevd in Flask 2.3.", 'flask_appbuilder'),
+    ("'_request_ctx_stack' is deprecated and will be removed in Flask 2.3.", 'flask_appbuilder'),
+    ("'_request_ctx_stack' is deprecated and will be removed in Flask 2.3.", 'flask_jwt_extended'),
 }
 
 KNOWN_COMMON_DEPRECATED_MESSAGES: Set[str] = {
@@ -215,9 +223,7 @@ KNOWN_DEPRECATED_DIRECT_IMPORTS: Set[str] = {
     "This module is deprecated. Please use `airflow.providers.microsoft.azure.sensors.cosmos`.",
     "This module is deprecated. Please use `airflow.providers.amazon.aws.hooks.dynamodb`.",
     "This module is deprecated. Please use `airflow.providers.microsoft.azure.transfers.local_to_wasb`.",
-    "This module is deprecated. Please use `airflow.providers.tableau.operators.tableau_refresh_workbook`.",
-    "This module is deprecated. Please use `airflow.providers.tableau.sensors.tableau_job_status`.",
-    "This module is deprecated. Please use `airflow.providers.tableau.hooks.tableau`.",
+    "This module is deprecated. Please use `airflow.providers.tableau.operators.tableau`.",
     "This module is deprecated. Please use `kubernetes.client.models.V1Volume`.",
     "This module is deprecated. Please use `kubernetes.client.models.V1VolumeMount`.",
     (
@@ -254,6 +260,7 @@ KNOWN_DEPRECATED_DIRECT_IMPORTS: Set[str] = {
     'This module is deprecated. Please use `airflow.providers.amazon.aws.sensors.redshift_cluster`.',
     "This module is deprecated. Please use airflow.providers.amazon.aws.transfers.sql_to_s3`.",
     "This module is deprecated. Please use `airflow.providers.tableau.sensors.tableau`.",
+    "This module is deprecated. Please use `airflow.providers.amazon.aws.operators.lambda_function`.",
 }
 
 
@@ -287,9 +294,9 @@ def get_all_providers() -> List[str]:
     Returns all providers for regular packages.
     :return: list of providers that are considered for provider packages
     """
-    from setup import PROVIDERS_REQUIREMENTS
+    from setup import ALL_PROVIDERS
 
-    return list(PROVIDERS_REQUIREMENTS.keys())
+    return list(ALL_PROVIDERS)
 
 
 def import_all_classes(
@@ -369,8 +376,8 @@ def import_all_classes(
             """
 [red]ERROR: There were some import errors[/]
 
-[yellow]If the job is about installing providers in 2.1.0, most likely you are using features that[/]
-[yellow]are not available in Airflow 2.1.0 and you mast implement them in backwards-compatible way![/]
+[yellow]If the job is about installing providers in 2.2.0, most likely you are using features that[/]
+[yellow]are not available in Airflow 2.2.0 and you must implement them in backwards-compatible way![/]
 
 """,
         )
@@ -384,16 +391,6 @@ def import_all_classes(
         return imported_classes, all_warnings
 
 
-def get_provider_packages() -> List[str]:
-    """
-    Returns all provider packages.
-
-    """
-    from setup import PROVIDERS_REQUIREMENTS
-
-    return list(PROVIDERS_REQUIREMENTS.keys())
-
-
 def is_imported_from_same_module(the_class: str, imported_name: str) -> bool:
     """
     Is the class imported from another module?
diff --git a/setup.cfg b/setup.cfg
index 2c96a0de42..8bfdaf713b 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -101,7 +101,10 @@ install_requires =
     cryptography>=0.9.3
     deprecated>=1.2.13
     dill>=0.2.2
-    flask>=2.0
+    # Flask 2.3 is scheduled to introduce a number of deprecation removals - some of them might be breaking
+    # for our dependencies - notably `_app_ctx_stack` and `_request_ctx_stack` removals.
+    # We should remove the limitation after 2.3 is released and our dependencies are updated to handle it
+    flask>=2.0,<2.3
     # We are tightly coupled with FAB version because we vendored in part of FAB code related to security manager
     # This is done as part of preparation to removing FAB as dependency, but we are not ready for it yet
     # Every time we update FAB version here, please make sure that you review the classes and models in
diff --git a/tests/www/views/test_views_custom_user_views.py b/tests/www/views/test_views_custom_user_views.py
index a0314726ea..4930caf444 100644
--- a/tests/www/views/test_views_custom_user_views.py
+++ b/tests/www/views/test_views_custom_user_views.py
@@ -34,22 +34,24 @@ class TestSecurity(unittest.TestCase):
     def setUpClass(cls):
         settings.configure_orm()
         cls.session = settings.Session
-        cls.app = application.create_app(testing=True)
-        cls.appbuilder = cls.app.appbuilder
-        cls.app.config['WTF_CSRF_ENABLED'] = False
-        cls.security_manager = cls.appbuilder.sm
-
-        cls.delete_roles()
 
     def setUp(self):
+        # We cannot reuse the app in tests (on class level) as in Flask 2.2 this causes
+        # an exception because app context teardown is removed and if even single request is run via app
+        # it cannot be re-intialized again by passing it as constructor to SQLA
+        # This makes the tests slightly slower (but they work with Flask 2.1 and 2.2
+        self.app = application.create_app(testing=True)
+        self.appbuilder = self.app.appbuilder
+        self.app.config['WTF_CSRF_ENABLED'] = False
+        self.security_manager = self.appbuilder.sm
+        self.delete_roles()
         self.db = SQLA(self.app)
         self.appbuilder.add_view(CustomUserDBModelView, "CustomUserDBModelView", category="ModelViews")
         self.client = self.app.test_client()  # type:ignore
 
-    @classmethod
-    def delete_roles(cls):
+    def delete_roles(self):
         for role_name in ['role_edit_one_dag']:
-            delete_role(cls.app, role_name)
+            delete_role(self.app, role_name)
 
     @parameterized.expand(
         [
diff --git a/tests/www/views/test_views_grid.py b/tests/www/views/test_views_grid.py
index a64a0452bb..412fb942dd 100644
--- a/tests/www/views/test_views_grid.py
+++ b/tests/www/views/test_views_grid.py
@@ -15,12 +15,12 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-
+from datetime import datetime, timedelta
 from typing import List
 
-import freezegun
 import pendulum
 import pytest
+from dateutil.tz import UTC
 
 from airflow.models import DagBag
 from airflow.models.dagrun import DagRun
@@ -34,7 +34,6 @@ from tests.test_utils.db import clear_db_runs
 from tests.test_utils.mock_operators import MockOperator
 
 DAG_ID = 'test'
-CURRENT_TIME = pendulum.DateTime(2021, 9, 7)
 
 
 @pytest.fixture(autouse=True, scope="module")
@@ -69,18 +68,17 @@ def dag_without_runs(dag_maker, session, app, monkeypatch):
 
 @pytest.fixture
 def dag_with_runs(dag_without_runs):
-    with freezegun.freeze_time(CURRENT_TIME):
-        date = dag_without_runs.dag.start_date
-        run_1 = dag_without_runs.create_dagrun(
-            run_id='run_1', state=DagRunState.SUCCESS, run_type=DagRunType.SCHEDULED, execution_date=date
-        )
-        run_2 = dag_without_runs.create_dagrun(
-            run_id='run_2',
-            run_type=DagRunType.SCHEDULED,
-            execution_date=dag_without_runs.dag.next_dagrun_info(date).logical_date,
-        )
+    date = dag_without_runs.dag.start_date
+    run_1 = dag_without_runs.create_dagrun(
+        run_id='run_1', state=DagRunState.SUCCESS, run_type=DagRunType.SCHEDULED, execution_date=date
+    )
+    run_2 = dag_without_runs.create_dagrun(
+        run_id='run_2',
+        run_type=DagRunType.SCHEDULED,
+        execution_date=dag_without_runs.dag.next_dagrun_info(date).logical_date,
+    )
 
-        yield run_1, run_2
+    yield run_1, run_2
 
 
 def test_no_runs(admin_client, dag_without_runs):
@@ -147,13 +145,23 @@ def test_one_run(admin_client, dag_with_runs: List[DagRun], session):
     session.flush()
 
     resp = admin_client.get(f'/object/grid_data?dag_id={DAG_ID}', follow_redirects=True)
+
     assert resp.status_code == 200, resp.json
-    assert resp.json == {
+
+    # We cannot use freezegun here as it does not play well with Flask 2.2 and SqlAlchemy
+    # Unlike real datetime, when FakeDatetime is used, it coerces to
+    # '2020-08-06 09:00:00+00:00' which is rejected by MySQL for EXPIRY Column
+    current_date_placeholder = '2022-01-02T00:00:00+00:00'
+    actual_date_in_json = datetime.fromisoformat(resp.json['dag_runs'][0]['end_date'])
+    assert datetime.now(tz=UTC) - actual_date_in_json < timedelta(minutes=5)
+    res = resp.json
+    res['dag_runs'][0]['end_date'] = current_date_placeholder
+    assert res == {
         'dag_runs': [
             {
                 'data_interval_end': '2016-01-02T00:00:00+00:00',
                 'data_interval_start': '2016-01-01T00:00:00+00:00',
-                'end_date': '2021-09-07T00:00:00+00:00',
+                'end_date': current_date_placeholder,
                 'execution_date': '2016-01-01T00:00:00+00:00',
                 'last_scheduling_decision': None,
                 'run_id': 'run_1',
diff --git a/tests/www/views/test_views_tasks.py b/tests/www/views/test_views_tasks.py
index d99a8fd5ca..5f6254f1fb 100644
--- a/tests/www/views/test_views_tasks.py
+++ b/tests/www/views/test_views_tasks.py
@@ -20,9 +20,9 @@ import json
 import re
 import unittest.mock
 import urllib.parse
+from datetime import timedelta
 
 import pytest
-from freezegun import freeze_time
 
 from airflow import settings
 from airflow.exceptions import AirflowException
@@ -546,7 +546,6 @@ def test_run_with_runnable_states(_, admin_client, session, state):
     'airflow.executors.executor_loader.ExecutorLoader.get_default_executor',
     return_value=_ForceHeartbeatCeleryExecutor(),
 )
-@freeze_time("2020-07-07 09:00:00")
 def test_run_ignoring_deps_sets_queued_dttm(_, admin_client, session):
     task_id = 'runme_0'
     session.query(TaskInstance).filter(TaskInstance.task_id == task_id).update(
@@ -566,10 +565,12 @@ def test_run_ignoring_deps_sets_queued_dttm(_, admin_client, session):
     resp = admin_client.post('run', data=form, follow_redirects=True)
 
     assert resp.status_code == 200
-
-    assert session.query(TaskInstance.queued_dttm).filter(TaskInstance.task_id == task_id).all() == [
-        (timezone.utcnow(),)
-    ]
+    # We cannot use freezegun here as it does not play well with Flask 2.2 and SqlAlchemy
+    # Unlike real datetime, when FakeDatetime is used, it coerces to
+    # '2020-08-06 09:00:00+00:00' which is rejected by MySQL for EXPIRY Column
+    assert timezone.utcnow() - session.query(TaskInstance.queued_dttm).filter(
+        TaskInstance.task_id == task_id
+    ).scalar() < timedelta(minutes=5)
 
 
 @pytest.mark.parametrize("state", QUEUEABLE_STATES)