Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress jaydebeapi.Error when setAutoCommit or getAutoCommit is unsupported by JDBC driver #38707

Merged
merged 37 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ff557a2
refactor: Suppress jaydebeapi.Error when setAutoCommit or getAutoComm…
davidblain-infrabel Apr 3, 2024
3e99705
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 3, 2024
1822f90
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 4, 2024
79b3c75
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 6, 2024
3a8bf82
refactor: Added suppress_and_warn context manager which allows specif…
davidblain-infrabel Apr 9, 2024
321ad02
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 9, 2024
ebbeffa
refactor: Print the stacktrace beside the exception when an exception…
davidblain-infrabel Apr 9, 2024
81fc3f6
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 9, 2024
1b37930
refactor: Re-organized imports of context
davidblain-infrabel Apr 9, 2024
0d8f4e2
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 9, 2024
da6a7d7
refactor: Added return type to suppress_and_warn contextmanager
davidblain-infrabel Apr 9, 2024
545c5d0
refactor: Use type instead of Type for Python 3.8 compat
davidblain-infrabel Apr 9, 2024
76e8e1f
refactor: Reorganized typing imports on context
davidblain-infrabel Apr 9, 2024
77c65c4
refactor: Added license on top of TestContext file
davidblain-infrabel Apr 9, 2024
b063da8
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 9, 2024
e02462a
refactor: Removed return type of suppress_and_warn
davidblain-infrabel Apr 9, 2024
72ac9d1
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 10, 2024
2b08af9
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 10, 2024
19e0df2
refactor: Moved suppress_and_warn method from Airflow context to comm…
davidblain-infrabel Apr 11, 2024
d0c61d0
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
569ef2c
refactor: Changed version of apache-airflow-providers-common-sql depe…
davidblain-infrabel Apr 11, 2024
2d2db8c
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
54b49a2
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
8eb0ee6
Revert "refactor: Changed version of apache-airflow-providers-common-…
davidblain-infrabel Apr 11, 2024
dcc02b6
refactor: Moved suppress_and_warn method from common sql provider to …
davidblain-infrabel Apr 11, 2024
c3807a7
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
63982cb
refactor: Also updated the apache-airflow dependency version from 2.6…
davidblain-infrabel Apr 11, 2024
36ec020
Revert "refactor: Also updated the apache-airflow dependency version …
davidblain-infrabel Apr 11, 2024
39b30da
fix: Move tests of suppress_and_warn from TestDbApiHook to TestJdbcHook
davidblain-infrabel Apr 11, 2024
522af2e
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
c212fc8
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
f8bfe3f
refactor: Updated common sql.pyi like main
davidblain-infrabel Apr 11, 2024
154052d
refactor: Return False in get_autocommit if exception is suppressed
davidblain-infrabel Apr 11, 2024
c531271
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
fdc3e2a
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
93618ef
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
77df3aa
Merge branch 'main' into feature/jdbc-autocommit
dabla Apr 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion airflow/providers/common/sql/hooks/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)


dabla marked this conversation as resolved.
Show resolved Hide resolved
class ConnectorProtocol(Protocol):
"""Database connection protocol."""

Expand Down
1 change: 1 addition & 0 deletions airflow/providers/common/sql/hooks/sql.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ 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: ...
Expand Down
8 changes: 5 additions & 3 deletions airflow/providers/jdbc/hooks/jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import jaydebeapi

from airflow.providers.common.sql.hooks.sql import DbApiHook
from airflow.providers.common.sql.hooks.sql import DbApiHook, suppress_and_warn

if TYPE_CHECKING:
from airflow.models.connection import Connection
Expand Down Expand Up @@ -152,7 +152,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_and_warn(jaydebeapi.Error):
conn.jconn.setAutoCommit(autocommit)
dabla marked this conversation as resolved.
Show resolved Hide resolved

def get_autocommit(self, conn: jaydebeapi.Connection) -> bool:
"""Get autocommit setting for the provided connection.
Expand All @@ -162,4 +163,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.
"""
return conn.jconn.getAutoCommit()
with suppress_and_warn(jaydebeapi.Error):
return conn.jconn.getAutoCommit()
2 changes: 1 addition & 1 deletion airflow/providers/jdbc/provider.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 12 additions & 2 deletions tests/providers/common/sql/hooks/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
dabla marked this conversation as resolved.
Show resolved Hide resolved
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()
15 changes: 15 additions & 0 deletions tests/providers/jdbc/hooks/test_jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from unittest import mock
from unittest.mock import Mock, patch

import jaydebeapi
import pytest

from airflow.models import Connection
Expand Down Expand Up @@ -82,13 +83,27 @@ 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()
jdbc_conn = jdbc_hook.get_conn()
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"
Expand Down
Loading