From ff557a2848a898138f33b26f7e527d7685f325ea Mon Sep 17 00:00:00 2001 From: David Blain Date: Wed, 3 Apr 2024 15:17:12 +0200 Subject: [PATCH 01/18] refactor: Suppress jaydebeapi.Error when setAutoCommit or getAutoCommit is being invoked as some JDBC drivers don't support this operation --- airflow/providers/jdbc/hooks/jdbc.py | 8 ++++++-- tests/providers/jdbc/hooks/test_jdbc.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/airflow/providers/jdbc/hooks/jdbc.py b/airflow/providers/jdbc/hooks/jdbc.py index dd592652ae0d3..2be40d392c012 100644 --- a/airflow/providers/jdbc/hooks/jdbc.py +++ b/airflow/providers/jdbc/hooks/jdbc.py @@ -17,6 +17,7 @@ # under the License. from __future__ import annotations +from contextlib import suppress from typing import TYPE_CHECKING, Any import jaydebeapi @@ -152,7 +153,8 @@ def set_autocommit(self, conn: jaydebeapi.Connection, autocommit: bool) -> None: :param conn: The connection. :param autocommit: The connection's autocommit setting. """ - conn.jconn.setAutoCommit(autocommit) + with suppress(jaydebeapi.Error): + conn.jconn.setAutoCommit(autocommit) def get_autocommit(self, conn: jaydebeapi.Connection) -> bool: """Get autocommit setting for the provided connection. @@ -162,4 +164,6 @@ def get_autocommit(self, conn: jaydebeapi.Connection) -> bool: to True on the connection. False if it is either not set, set to False, or the connection does not support auto-commit. """ - return conn.jconn.getAutoCommit() + with suppress(jaydebeapi.Error): + return conn.jconn.getAutoCommit() + return False diff --git a/tests/providers/jdbc/hooks/test_jdbc.py b/tests/providers/jdbc/hooks/test_jdbc.py index 8b303db67e2fc..4bd5ebdc92118 100644 --- a/tests/providers/jdbc/hooks/test_jdbc.py +++ b/tests/providers/jdbc/hooks/test_jdbc.py @@ -22,6 +22,7 @@ from unittest import mock from unittest.mock import Mock, patch +import jaydebeapi import pytest from airflow.models import Connection @@ -82,6 +83,13 @@ def test_jdbc_conn_set_autocommit(self, _): jdbc_hook.set_autocommit(jdbc_conn, False) jdbc_conn.jconn.setAutoCommit.assert_called_once_with(False) + @patch("airflow.providers.jdbc.hooks.jdbc.jaydebeapi.connect") + def test_jdbc_conn_set_autocommit_when_not_supported(self, _): + jdbc_hook = JdbcHook() + jdbc_conn = jdbc_hook.get_conn() + jdbc_conn.jconn.setAutoCommit.side_effect = jaydebeapi.Error() + jdbc_hook.set_autocommit(jdbc_conn, False) + @patch("airflow.providers.jdbc.hooks.jdbc.jaydebeapi.connect") def test_jdbc_conn_get_autocommit(self, _): jdbc_hook = JdbcHook() @@ -89,6 +97,13 @@ def test_jdbc_conn_get_autocommit(self, _): jdbc_hook.get_autocommit(jdbc_conn) jdbc_conn.jconn.getAutoCommit.assert_called_once_with() + @patch("airflow.providers.jdbc.hooks.jdbc.jaydebeapi.connect") + def test_jdbc_conn_get_autocommit_when_not_supported_then_return_false(self, _): + jdbc_hook = JdbcHook() + jdbc_conn = jdbc_hook.get_conn() + jdbc_conn.jconn.getAutoCommit.side_effect = jaydebeapi.Error() + assert jdbc_hook.get_autocommit(jdbc_conn) is False + def test_driver_hook_params(self): hook = get_hook(hook_params=dict(driver_path="Blah driver path", driver_class="Blah driver class")) assert hook.driver_path == "Blah driver path" From 3a8bf824f3dfcd76ceb2112ad9ab939fbc4b2087 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 10:30:05 +0200 Subject: [PATCH 02/18] refactor: Added suppress_and_warn context manager which allows specifying the exception to be suppressed and log a warning --- airflow/providers/jdbc/hooks/jdbc.py | 7 +++---- airflow/utils/context.py | 11 ++++++++++- airflow/utils/context.pyi | 3 ++- tests/utils/test_context.py | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 tests/utils/test_context.py diff --git a/airflow/providers/jdbc/hooks/jdbc.py b/airflow/providers/jdbc/hooks/jdbc.py index 2be40d392c012..4cce1fb6b5b48 100644 --- a/airflow/providers/jdbc/hooks/jdbc.py +++ b/airflow/providers/jdbc/hooks/jdbc.py @@ -17,12 +17,12 @@ # under the License. from __future__ import annotations -from contextlib import suppress from typing import TYPE_CHECKING, Any import jaydebeapi from airflow.providers.common.sql.hooks.sql import DbApiHook +from airflow.utils.context import suppress_and_warn if TYPE_CHECKING: from airflow.models.connection import Connection @@ -153,7 +153,7 @@ def set_autocommit(self, conn: jaydebeapi.Connection, autocommit: bool) -> None: :param conn: The connection. :param autocommit: The connection's autocommit setting. """ - with suppress(jaydebeapi.Error): + with suppress_and_warn(jaydebeapi.Error): conn.jconn.setAutoCommit(autocommit) def get_autocommit(self, conn: jaydebeapi.Connection) -> bool: @@ -164,6 +164,5 @@ def get_autocommit(self, conn: jaydebeapi.Connection) -> bool: to True on the connection. False if it is either not set, set to False, or the connection does not support auto-commit. """ - with suppress(jaydebeapi.Error): + with suppress_and_warn(jaydebeapi.Error): return conn.jconn.getAutoCommit() - return False diff --git a/airflow/utils/context.py b/airflow/utils/context.py index 033b7aa39d3ba..99dbd72cdac25 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -33,7 +33,7 @@ Mapping, MutableMapping, SupportsIndex, - ValuesView, + ValuesView, Type, ) import attrs @@ -361,3 +361,12 @@ def _create_value(k: str, v: Any) -> Any: return lazy_object_proxy.Proxy(factory) return {k: _create_value(k, v) for k, v in source._context.items()} + + +@contextlib.contextmanager +def suppress_and_warn(*exceptions: Type[BaseException]): + """Context manager that suppresses the given exceptions and logs a warning message.""" + try: + yield + except exceptions as e: + warnings.warn(f"Exception suppressed: {e}") diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index 8b5deb4746918..4df8197be4137 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -26,7 +26,7 @@ # declare "these are defined, but don't error if others are accessed" someday. from __future__ import annotations -from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload +from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Type from pendulum import DateTime @@ -127,3 +127,4 @@ def context_merge(context: Context, **kwargs: Any) -> None: ... def context_update_for_unmapped(context: Context, task: BaseOperator) -> None: ... def context_copy_partial(source: Context, keys: Container[str]) -> Context: ... def lazy_mapping_from_context(source: Context) -> Mapping[str, Any]: ... +def suppress_and_warn(*exceptions: Type[BaseException]): ... diff --git a/tests/utils/test_context.py b/tests/utils/test_context.py new file mode 100644 index 0000000000000..7b4aa9f94ccb7 --- /dev/null +++ b/tests/utils/test_context.py @@ -0,0 +1,19 @@ +import pytest + +from airflow.exceptions import DeserializingResultError +from airflow.utils.context import AirflowContextDeprecationWarning, suppress_and_warn + + +def method_which_raises_an_airflow_deprecation_warning(): + raise AirflowContextDeprecationWarning() + + +class TestContext: + def test_suppress_and_warn_when_raised_exception_is_suppressed(self): + with suppress_and_warn(AirflowContextDeprecationWarning): + method_which_raises_an_airflow_deprecation_warning() + + def test_suppress_and_warn_when_raised_exception_is_not_suppressed(self): + with pytest.raises(AirflowContextDeprecationWarning): + with suppress_and_warn(DeserializingResultError): + method_which_raises_an_airflow_deprecation_warning() From ebbeffad25f3a2df178afe584ec1533b636b9087 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 11:09:13 +0200 Subject: [PATCH 03/18] refactor: Print the stacktrace beside the exception when an exception is suppressed --- airflow/utils/context.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index 8c08e76f107c2..fa0bdeaf036da 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -22,6 +22,7 @@ import contextlib import copy import functools +import traceback import warnings from typing import ( TYPE_CHECKING, @@ -357,12 +358,13 @@ def _create_value(k: str, v: Any) -> Any: return {k: _create_value(k, v) for k, v in source._context.items()} +@contextlib.contextmanager def suppress_and_warn(*exceptions: Type[BaseException]): """Context manager that suppresses the given exceptions and logs a warning message.""" try: yield except exceptions as e: - warnings.warn(f"Exception suppressed: {e}") + warnings.warn(f"Exception suppressed: {e}\n{traceback.format_exc()}", category=UserWarning) def context_get_dataset_events(context: Context) -> DatasetEventAccessors: From 1b379300b98e7d12c250832aec5b19726cbb3497 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 16:12:48 +0200 Subject: [PATCH 04/18] refactor: Re-organized imports of context --- airflow/utils/context.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index fa0bdeaf036da..9433223a459ac 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -34,7 +34,8 @@ Mapping, MutableMapping, SupportsIndex, - ValuesView, Type, + Type, + ValuesView, ) import attrs From da6a7d79710d45f94be08d312eb98f6a38c0d51f Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 16:22:56 +0200 Subject: [PATCH 05/18] refactor: Added return type to suppress_and_warn contextmanager --- airflow/utils/context.py | 4 ++-- airflow/utils/context.pyi | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index 9433223a459ac..6cc5af9023824 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -35,7 +35,7 @@ MutableMapping, SupportsIndex, Type, - ValuesView, + ValuesView, Generator, ) import attrs @@ -360,7 +360,7 @@ def _create_value(k: str, v: Any) -> Any: @contextlib.contextmanager -def suppress_and_warn(*exceptions: Type[BaseException]): +def suppress_and_warn(*exceptions: Type[BaseException]) -> Generator[Type[BaseException], None, None]: """Context manager that suppresses the given exceptions and logs a warning message.""" try: yield diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index 3c1ac224d6b51..527e2cb227e42 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -26,7 +26,7 @@ # declare "these are defined, but don't error if others are accessed" someday. from __future__ import annotations -from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Type +from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Type, Generator from pendulum import DateTime @@ -127,5 +127,5 @@ def context_merge(context: Context, **kwargs: Any) -> None: ... def context_update_for_unmapped(context: Context, task: BaseOperator) -> None: ... def context_copy_partial(source: Context, keys: Container[str]) -> Context: ... def lazy_mapping_from_context(source: Context) -> Mapping[str, Any]: ... -def suppress_and_warn(*exceptions: Type[BaseException]): ... +def suppress_and_warn(*exceptions: Type[BaseException]) -> Generator[Type[BaseException], None, None]: ... def context_get_dataset_events(context: Context) -> DatasetEventAccessors: ... From 545c5d05cb42fd72e4e4aca210b7e2358ca762ac Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 16:49:53 +0200 Subject: [PATCH 06/18] refactor: Use type instead of Type for Python 3.8 compat --- airflow/utils/context.py | 3 +-- airflow/utils/context.pyi | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index 6cc5af9023824..e907f83621d80 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -34,7 +34,6 @@ Mapping, MutableMapping, SupportsIndex, - Type, ValuesView, Generator, ) @@ -360,7 +359,7 @@ def _create_value(k: str, v: Any) -> Any: @contextlib.contextmanager -def suppress_and_warn(*exceptions: Type[BaseException]) -> Generator[Type[BaseException], None, None]: +def suppress_and_warn(*exceptions: type[BaseException]) -> Generator[type[BaseException], None, None]: """Context manager that suppresses the given exceptions and logs a warning message.""" try: yield diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index 527e2cb227e42..a6e5b1c95a60a 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -26,7 +26,7 @@ # declare "these are defined, but don't error if others are accessed" someday. from __future__ import annotations -from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Type, Generator +from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Generator from pendulum import DateTime @@ -127,5 +127,5 @@ def context_merge(context: Context, **kwargs: Any) -> None: ... def context_update_for_unmapped(context: Context, task: BaseOperator) -> None: ... def context_copy_partial(source: Context, keys: Container[str]) -> Context: ... def lazy_mapping_from_context(source: Context) -> Mapping[str, Any]: ... -def suppress_and_warn(*exceptions: Type[BaseException]) -> Generator[Type[BaseException], None, None]: ... +def suppress_and_warn(*exceptions: type[BaseException]) -> Generator[type[BaseException], None, None]: ... def context_get_dataset_events(context: Context) -> DatasetEventAccessors: ... From 76e8e1f56c0148e83438013578fa4af137666531 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 16:50:51 +0200 Subject: [PATCH 07/18] refactor: Reorganized typing imports on context --- airflow/utils/context.py | 3 ++- airflow/utils/context.pyi | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index e907f83621d80..a2f7d9ce7181f 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -28,13 +28,14 @@ TYPE_CHECKING, Any, Container, + Generator, ItemsView, Iterator, KeysView, Mapping, MutableMapping, SupportsIndex, - ValuesView, Generator, + ValuesView, ) import attrs diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index a6e5b1c95a60a..e9d4246af3f37 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -26,7 +26,7 @@ # declare "these are defined, but don't error if others are accessed" someday. from __future__ import annotations -from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload, Generator +from typing import Any, Collection, Container, Generator, Iterable, Iterator, Mapping, overload from pendulum import DateTime From 77c65c44e14017440dd8121a5d315a4bec1271d2 Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 16:52:21 +0200 Subject: [PATCH 08/18] refactor: Added license on top of TestContext file --- tests/utils/test_context.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/utils/test_context.py b/tests/utils/test_context.py index 7b4aa9f94ccb7..38794414f9b1c 100644 --- a/tests/utils/test_context.py +++ b/tests/utils/test_context.py @@ -1,3 +1,21 @@ +# 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. +from __future__ import annotations + import pytest from airflow.exceptions import DeserializingResultError From e02462a3993cd7dfa28610b60bd914488eaa39ea Mon Sep 17 00:00:00 2001 From: David Blain Date: Tue, 9 Apr 2024 17:17:10 +0200 Subject: [PATCH 09/18] refactor: Removed return type of suppress_and_warn --- airflow/utils/context.py | 3 +-- airflow/utils/context.pyi | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/airflow/utils/context.py b/airflow/utils/context.py index a2f7d9ce7181f..82d4a122eb574 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -28,7 +28,6 @@ TYPE_CHECKING, Any, Container, - Generator, ItemsView, Iterator, KeysView, @@ -360,7 +359,7 @@ def _create_value(k: str, v: Any) -> Any: @contextlib.contextmanager -def suppress_and_warn(*exceptions: type[BaseException]) -> Generator[type[BaseException], None, None]: +def suppress_and_warn(*exceptions: type[BaseException]): """Context manager that suppresses the given exceptions and logs a warning message.""" try: yield diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index e9d4246af3f37..2c81506be9ece 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -26,7 +26,7 @@ # declare "these are defined, but don't error if others are accessed" someday. from __future__ import annotations -from typing import Any, Collection, Container, Generator, Iterable, Iterator, Mapping, overload +from typing import Any, Collection, Container, Iterable, Iterator, Mapping, overload from pendulum import DateTime @@ -127,5 +127,5 @@ def context_merge(context: Context, **kwargs: Any) -> None: ... def context_update_for_unmapped(context: Context, task: BaseOperator) -> None: ... def context_copy_partial(source: Context, keys: Container[str]) -> Context: ... def lazy_mapping_from_context(source: Context) -> Mapping[str, Any]: ... -def suppress_and_warn(*exceptions: type[BaseException]) -> Generator[type[BaseException], None, None]: ... +def suppress_and_warn(*exceptions: type[BaseException]): ... def context_get_dataset_events(context: Context) -> DatasetEventAccessors: ... From 19e0df2205f07c9afa15020ebef5e028f19b143a Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 07:59:27 +0200 Subject: [PATCH 10/18] refactor: Moved suppress_and_warn method from Airflow context to common sql provider --- airflow/providers/common/sql/hooks/sql.py | 12 ++++++- airflow/providers/common/sql/hooks/sql.pyi | 1 + airflow/providers/jdbc/hooks/jdbc.py | 3 +- airflow/utils/context.py | 10 ------ airflow/utils/context.pyi | 1 - tests/providers/common/sql/hooks/test_sql.py | 14 ++++++-- tests/utils/test_context.py | 37 -------------------- 7 files changed, 25 insertions(+), 53 deletions(-) delete mode 100644 tests/utils/test_context.py diff --git a/airflow/providers/common/sql/hooks/sql.py b/airflow/providers/common/sql/hooks/sql.py index 7f1536a39b47e..b586c53295bb2 100644 --- a/airflow/providers/common/sql/hooks/sql.py +++ b/airflow/providers/common/sql/hooks/sql.py @@ -17,8 +17,9 @@ from __future__ import annotations import contextlib +import traceback import warnings -from contextlib import closing +from contextlib import closing, contextmanager from datetime import datetime from typing import ( TYPE_CHECKING, @@ -111,6 +112,15 @@ def fetch_one_handler(cursor) -> list[tuple] | None: return None +@contextmanager +def suppress_and_warn(*exceptions: type[BaseException]): + """Context manager that suppresses the given exceptions and logs a warning message.""" + try: + yield + except exceptions as e: + warnings.warn(f"Exception suppressed: {e}\n{traceback.format_exc()}", category=UserWarning) + + class ConnectorProtocol(Protocol): """Database connection protocol.""" diff --git a/airflow/providers/common/sql/hooks/sql.pyi b/airflow/providers/common/sql/hooks/sql.pyi index 08e8553a68d4d..06c9221f12cfd 100644 --- a/airflow/providers/common/sql/hooks/sql.pyi +++ b/airflow/providers/common/sql/hooks/sql.pyi @@ -41,6 +41,7 @@ def return_single_query_results( ): ... def fetch_all_handler(cursor) -> Union[list[tuple], None]: ... def fetch_one_handler(cursor) -> Union[list[tuple], None]: ... +def suppress_and_warn(*exceptions: type[BaseException]): ... class ConnectorProtocol(Protocol): def connect(self, host: str, port: int, username: str, schema: str) -> Any: ... diff --git a/airflow/providers/jdbc/hooks/jdbc.py b/airflow/providers/jdbc/hooks/jdbc.py index 4cce1fb6b5b48..29f3465a58a34 100644 --- a/airflow/providers/jdbc/hooks/jdbc.py +++ b/airflow/providers/jdbc/hooks/jdbc.py @@ -21,8 +21,7 @@ import jaydebeapi -from airflow.providers.common.sql.hooks.sql import DbApiHook -from airflow.utils.context import suppress_and_warn +from airflow.providers.common.sql.hooks.sql import DbApiHook, suppress_and_warn if TYPE_CHECKING: from airflow.models.connection import Connection diff --git a/airflow/utils/context.py b/airflow/utils/context.py index 82d4a122eb574..78536bc97222a 100644 --- a/airflow/utils/context.py +++ b/airflow/utils/context.py @@ -22,7 +22,6 @@ import contextlib import copy import functools -import traceback import warnings from typing import ( TYPE_CHECKING, @@ -358,15 +357,6 @@ def _create_value(k: str, v: Any) -> Any: return {k: _create_value(k, v) for k, v in source._context.items()} -@contextlib.contextmanager -def suppress_and_warn(*exceptions: type[BaseException]): - """Context manager that suppresses the given exceptions and logs a warning message.""" - try: - yield - except exceptions as e: - warnings.warn(f"Exception suppressed: {e}\n{traceback.format_exc()}", category=UserWarning) - - def context_get_dataset_events(context: Context) -> DatasetEventAccessors: try: return context["dataset_events"] diff --git a/airflow/utils/context.pyi b/airflow/utils/context.pyi index 2c81506be9ece..eb2cf6dd3e46f 100644 --- a/airflow/utils/context.pyi +++ b/airflow/utils/context.pyi @@ -127,5 +127,4 @@ def context_merge(context: Context, **kwargs: Any) -> None: ... def context_update_for_unmapped(context: Context, task: BaseOperator) -> None: ... def context_copy_partial(source: Context, keys: Container[str]) -> Context: ... def lazy_mapping_from_context(source: Context) -> Mapping[str, Any]: ... -def suppress_and_warn(*exceptions: type[BaseException]): ... def context_get_dataset_events(context: Context) -> DatasetEventAccessors: ... diff --git a/tests/providers/common/sql/hooks/test_sql.py b/tests/providers/common/sql/hooks/test_sql.py index 4bd5bdcc549f3..1b32d096ffa92 100644 --- a/tests/providers/common/sql/hooks/test_sql.py +++ b/tests/providers/common/sql/hooks/test_sql.py @@ -23,9 +23,10 @@ import pytest -from airflow.exceptions import AirflowProviderDeprecationWarning +from airflow.exceptions import AirflowProviderDeprecationWarning, DeserializingResultError from airflow.models import Connection -from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler +from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, suppress_and_warn +from airflow.utils.context import AirflowContextDeprecationWarning from airflow.utils.session import provide_session from tests.providers.common.sql.test_utils import mock_hook @@ -251,3 +252,12 @@ def test_make_common_data_structure_no_deprecated_method(self): def test_placeholder_config_from_extra(self): dbapi_hook = mock_hook(DbApiHook, conn_params={"extra": {"placeholder": "?"}}) assert dbapi_hook.placeholder == "?" + + def test_suppress_and_warn_when_raised_exception_is_suppressed(self): + with suppress_and_warn(AirflowContextDeprecationWarning): + raise AirflowContextDeprecationWarning() + + def test_suppress_and_warn_when_raised_exception_is_not_suppressed(self): + with pytest.raises(AirflowContextDeprecationWarning): + with suppress_and_warn(DeserializingResultError): + raise AirflowContextDeprecationWarning() diff --git a/tests/utils/test_context.py b/tests/utils/test_context.py deleted file mode 100644 index 38794414f9b1c..0000000000000 --- a/tests/utils/test_context.py +++ /dev/null @@ -1,37 +0,0 @@ -# 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. -from __future__ import annotations - -import pytest - -from airflow.exceptions import DeserializingResultError -from airflow.utils.context import AirflowContextDeprecationWarning, suppress_and_warn - - -def method_which_raises_an_airflow_deprecation_warning(): - raise AirflowContextDeprecationWarning() - - -class TestContext: - def test_suppress_and_warn_when_raised_exception_is_suppressed(self): - with suppress_and_warn(AirflowContextDeprecationWarning): - method_which_raises_an_airflow_deprecation_warning() - - def test_suppress_and_warn_when_raised_exception_is_not_suppressed(self): - with pytest.raises(AirflowContextDeprecationWarning): - with suppress_and_warn(DeserializingResultError): - method_which_raises_an_airflow_deprecation_warning() From 569ef2c2dae8c9c1c5598f65ff426cb13012af15 Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 08:10:50 +0200 Subject: [PATCH 11/18] refactor: Changed version of apache-airflow-providers-common-sql dependency to be at least 1.12.0 so the jdbc provider is able to use the newly added suppress_and_warn method --- airflow/providers/jdbc/provider.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/providers/jdbc/provider.yaml b/airflow/providers/jdbc/provider.yaml index 912f669865e8d..2e146afc205f6 100644 --- a/airflow/providers/jdbc/provider.yaml +++ b/airflow/providers/jdbc/provider.yaml @@ -49,7 +49,7 @@ versions: dependencies: - apache-airflow>=2.6.0 - - apache-airflow-providers-common-sql>=1.3.1 + - apache-airflow-providers-common-sql>=1.12.0 - jaydebeapi>=1.1.1 integrations: From 8eb0ee6bf95d982c00cfe17443741f2a15ddc8d6 Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 11:58:13 +0200 Subject: [PATCH 12/18] Revert "refactor: Changed version of apache-airflow-providers-common-sql dependency to be at least 1.12.0 so the jdbc provider is able to use the newly added suppress_and_warn method" This reverts commit 569ef2c2dae8c9c1c5598f65ff426cb13012af15. --- airflow/providers/jdbc/provider.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/providers/jdbc/provider.yaml b/airflow/providers/jdbc/provider.yaml index 2e146afc205f6..912f669865e8d 100644 --- a/airflow/providers/jdbc/provider.yaml +++ b/airflow/providers/jdbc/provider.yaml @@ -49,7 +49,7 @@ versions: dependencies: - apache-airflow>=2.6.0 - - apache-airflow-providers-common-sql>=1.12.0 + - apache-airflow-providers-common-sql>=1.3.1 - jaydebeapi>=1.1.1 integrations: From dcc02b6c7b1801ff35be7dad412a0faed7f7252a Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 12:00:40 +0200 Subject: [PATCH 13/18] refactor: Moved suppress_and_warn method from common sql provider to jdbc provider --- airflow/providers/common/sql/hooks/sql.py | 12 +----------- airflow/providers/common/sql/hooks/sql.pyi | 12 ++++-------- airflow/providers/jdbc/hooks/jdbc.py | 14 +++++++++++++- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/airflow/providers/common/sql/hooks/sql.py b/airflow/providers/common/sql/hooks/sql.py index b586c53295bb2..7f1536a39b47e 100644 --- a/airflow/providers/common/sql/hooks/sql.py +++ b/airflow/providers/common/sql/hooks/sql.py @@ -17,9 +17,8 @@ from __future__ import annotations import contextlib -import traceback import warnings -from contextlib import closing, contextmanager +from contextlib import closing from datetime import datetime from typing import ( TYPE_CHECKING, @@ -112,15 +111,6 @@ def fetch_one_handler(cursor) -> list[tuple] | None: return None -@contextmanager -def suppress_and_warn(*exceptions: type[BaseException]): - """Context manager that suppresses the given exceptions and logs a warning message.""" - try: - yield - except exceptions as e: - warnings.warn(f"Exception suppressed: {e}\n{traceback.format_exc()}", category=UserWarning) - - class ConnectorProtocol(Protocol): """Database connection protocol.""" diff --git a/airflow/providers/common/sql/hooks/sql.pyi b/airflow/providers/common/sql/hooks/sql.pyi index 119d37169d48d..5adf2dadf5b40 100644 --- a/airflow/providers/common/sql/hooks/sql.pyi +++ b/airflow/providers/common/sql/hooks/sql.pyi @@ -32,16 +32,13 @@ Definition of the public interface for airflow.providers.common.sql.hooks.sql isort:skip_file """ from _typeshed import Incomplete -from airflow.exceptions import ( - AirflowException as AirflowException, - AirflowOptionalProviderFeatureException as AirflowOptionalProviderFeatureException, - AirflowProviderDeprecationWarning as AirflowProviderDeprecationWarning, -) +from typing import Any, Callable, Generator, Iterable, Mapping, Protocol, Sequence, TypeVar, overload + +from pandas import DataFrame as DataFrame + from airflow.hooks.base import BaseHook as BaseHook from airflow.providers.openlineage.extractors import OperatorLineage as OperatorLineage from airflow.providers.openlineage.sqlparser import DatabaseInfo as DatabaseInfo -from pandas import DataFrame as DataFrame -from typing import Any, Callable, Generator, Iterable, Mapping, Protocol, Sequence, TypeVar, overload T = TypeVar("T") SQL_PLACEHOLDERS: Incomplete @@ -49,7 +46,6 @@ SQL_PLACEHOLDERS: Incomplete def return_single_query_results(sql: str | Iterable[str], return_last: bool, split_statements: bool): ... def fetch_all_handler(cursor) -> list[tuple] | None: ... def fetch_one_handler(cursor) -> list[tuple] | None: ... -def suppress_and_warn(*exceptions: type[BaseException]): ... class ConnectorProtocol(Protocol): def connect(self, host: str, port: int, username: str, schema: str) -> Any: ... diff --git a/airflow/providers/jdbc/hooks/jdbc.py b/airflow/providers/jdbc/hooks/jdbc.py index 29f3465a58a34..62b2f04505531 100644 --- a/airflow/providers/jdbc/hooks/jdbc.py +++ b/airflow/providers/jdbc/hooks/jdbc.py @@ -17,16 +17,28 @@ # under the License. from __future__ import annotations +import traceback +import warnings +from contextlib import contextmanager from typing import TYPE_CHECKING, Any import jaydebeapi -from airflow.providers.common.sql.hooks.sql import DbApiHook, suppress_and_warn +from airflow.providers.common.sql.hooks.sql import DbApiHook if TYPE_CHECKING: from airflow.models.connection import Connection +@contextmanager +def suppress_and_warn(*exceptions: type[BaseException]): + """Context manager that suppresses the given exceptions and logs a warning message.""" + try: + yield + except exceptions as e: + warnings.warn(f"Exception suppressed: {e}\n{traceback.format_exc()}", category=UserWarning) + + class JdbcHook(DbApiHook): """General hook for JDBC access. From 63982cbd398502035238be3f5c60691ab3ab6f81 Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 13:13:08 +0200 Subject: [PATCH 14/18] refactor: Also updated the apache-airflow dependency version from 2.6.0 to 2.7.0 for microsoft-azure provider in provider_dependencies.json --- generated/provider_dependencies.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generated/provider_dependencies.json b/generated/provider_dependencies.json index 9315766f81926..8122ef72fecd7 100644 --- a/generated/provider_dependencies.json +++ b/generated/provider_dependencies.json @@ -680,7 +680,7 @@ "deps": [ "adal>=1.2.7", "adlfs>=2023.10.0", - "apache-airflow>=2.6.0", + "apache-airflow>=2.7.0", "azure-batch>=8.0.0", "azure-cosmos>=4.6.0", "azure-datalake-store>=0.0.45", From 36ec0202fc8a9253fc809abbbfaeb281af936d69 Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 13:58:46 +0200 Subject: [PATCH 15/18] Revert "refactor: Also updated the apache-airflow dependency version from 2.6.0 to 2.7.0 for microsoft-azure provider in provider_dependencies.json" This reverts commit 63982cbd398502035238be3f5c60691ab3ab6f81. --- generated/provider_dependencies.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generated/provider_dependencies.json b/generated/provider_dependencies.json index 8122ef72fecd7..9315766f81926 100644 --- a/generated/provider_dependencies.json +++ b/generated/provider_dependencies.json @@ -680,7 +680,7 @@ "deps": [ "adal>=1.2.7", "adlfs>=2023.10.0", - "apache-airflow>=2.7.0", + "apache-airflow>=2.6.0", "azure-batch>=8.0.0", "azure-cosmos>=4.6.0", "azure-datalake-store>=0.0.45", From 39b30da3ef981ae6bdff09817f59a4f1c6045348 Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 14:02:49 +0200 Subject: [PATCH 16/18] fix: Move tests of suppress_and_warn from TestDbApiHook to TestJdbcHook --- tests/providers/common/sql/hooks/test_sql.py | 14 ++------------ tests/providers/jdbc/hooks/test_jdbc.py | 13 ++++++++++++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/providers/common/sql/hooks/test_sql.py b/tests/providers/common/sql/hooks/test_sql.py index 1b32d096ffa92..4bd5bdcc549f3 100644 --- a/tests/providers/common/sql/hooks/test_sql.py +++ b/tests/providers/common/sql/hooks/test_sql.py @@ -23,10 +23,9 @@ import pytest -from airflow.exceptions import AirflowProviderDeprecationWarning, DeserializingResultError +from airflow.exceptions import AirflowProviderDeprecationWarning from airflow.models import Connection -from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, suppress_and_warn -from airflow.utils.context import AirflowContextDeprecationWarning +from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler from airflow.utils.session import provide_session from tests.providers.common.sql.test_utils import mock_hook @@ -252,12 +251,3 @@ def test_make_common_data_structure_no_deprecated_method(self): def test_placeholder_config_from_extra(self): dbapi_hook = mock_hook(DbApiHook, conn_params={"extra": {"placeholder": "?"}}) assert dbapi_hook.placeholder == "?" - - def test_suppress_and_warn_when_raised_exception_is_suppressed(self): - with suppress_and_warn(AirflowContextDeprecationWarning): - raise AirflowContextDeprecationWarning() - - def test_suppress_and_warn_when_raised_exception_is_not_suppressed(self): - with pytest.raises(AirflowContextDeprecationWarning): - with suppress_and_warn(DeserializingResultError): - raise AirflowContextDeprecationWarning() diff --git a/tests/providers/jdbc/hooks/test_jdbc.py b/tests/providers/jdbc/hooks/test_jdbc.py index 4bd5ebdc92118..80eedf8ee47be 100644 --- a/tests/providers/jdbc/hooks/test_jdbc.py +++ b/tests/providers/jdbc/hooks/test_jdbc.py @@ -25,9 +25,11 @@ import jaydebeapi import pytest +from airflow.exceptions import DeserializingResultError from airflow.models import Connection -from airflow.providers.jdbc.hooks.jdbc import JdbcHook +from airflow.providers.jdbc.hooks.jdbc import JdbcHook, suppress_and_warn from airflow.utils import db +from airflow.utils.context import AirflowContextDeprecationWarning pytestmark = pytest.mark.db_test @@ -176,3 +178,12 @@ def test_driver_extra_raises_warning_and_returns_default_driver_by_default(self, "have supplied 'driver_class' via connection extra but it will not be used" ) in caplog.text assert driver_class == "Blah driver class" + + def test_suppress_and_warn_when_raised_exception_is_suppressed(self): + with suppress_and_warn(AirflowContextDeprecationWarning): + raise AirflowContextDeprecationWarning() + + def test_suppress_and_warn_when_raised_exception_is_not_suppressed(self): + with pytest.raises(AirflowContextDeprecationWarning): + with suppress_and_warn(DeserializingResultError): + raise AirflowContextDeprecationWarning() From f8bfe3fc3743e91fd4f225e5ad076e4dbfba1f4a Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 14:35:16 +0200 Subject: [PATCH 17/18] refactor: Updated common sql.pyi like main --- airflow/providers/common/sql/hooks/sql.pyi | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/airflow/providers/common/sql/hooks/sql.pyi b/airflow/providers/common/sql/hooks/sql.pyi index 5adf2dadf5b40..83135a235bf97 100644 --- a/airflow/providers/common/sql/hooks/sql.pyi +++ b/airflow/providers/common/sql/hooks/sql.pyi @@ -32,13 +32,16 @@ Definition of the public interface for airflow.providers.common.sql.hooks.sql isort:skip_file """ from _typeshed import Incomplete -from typing import Any, Callable, Generator, Iterable, Mapping, Protocol, Sequence, TypeVar, overload - -from pandas import DataFrame as DataFrame - +from airflow.exceptions import ( + AirflowException as AirflowException, + AirflowOptionalProviderFeatureException as AirflowOptionalProviderFeatureException, + AirflowProviderDeprecationWarning as AirflowProviderDeprecationWarning, +) from airflow.hooks.base import BaseHook as BaseHook from airflow.providers.openlineage.extractors import OperatorLineage as OperatorLineage from airflow.providers.openlineage.sqlparser import DatabaseInfo as DatabaseInfo +from pandas import DataFrame as DataFrame +from typing import Any, Callable, Generator, Iterable, Mapping, Protocol, Sequence, TypeVar, overload T = TypeVar("T") SQL_PLACEHOLDERS: Incomplete From 154052dcdb8069a531844872d36691ab58c409aa Mon Sep 17 00:00:00 2001 From: David Blain Date: Thu, 11 Apr 2024 16:35:26 +0200 Subject: [PATCH 18/18] refactor: Return False in get_autocommit if exception is suppressed --- airflow/providers/jdbc/hooks/jdbc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/airflow/providers/jdbc/hooks/jdbc.py b/airflow/providers/jdbc/hooks/jdbc.py index 62b2f04505531..832c16b9aad48 100644 --- a/airflow/providers/jdbc/hooks/jdbc.py +++ b/airflow/providers/jdbc/hooks/jdbc.py @@ -177,3 +177,4 @@ def get_autocommit(self, conn: jaydebeapi.Connection) -> bool: """ with suppress_and_warn(jaydebeapi.Error): return conn.jconn.getAutoCommit() + return False