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

Undeprecating DBApiHookForTests._make_common_data_structure #38573

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 8 additions & 9 deletions airflow/providers/common/sql/hooks/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from __future__ import annotations

import contextlib
import warnings
from contextlib import closing
from datetime import datetime
from typing import (
Expand All @@ -36,7 +37,6 @@
from urllib.parse import urlparse

import sqlparse
from deprecated import deprecated
from more_itertools import chunked
from sqlalchemy import create_engine

Expand Down Expand Up @@ -422,14 +422,6 @@ def run(
else:
return results

@deprecated(
reason=(
"The `_make_serializable` method is deprecated and support will be removed in a future "
"version of the common.sql provider. Please update the DbApiHook's provider "
"to a version based on common.sql >= 1.9.1."
),
category=AirflowProviderDeprecationWarning,
)
def _make_common_data_structure(self, result: T | Sequence[T]) -> tuple | list[tuple]:
"""Ensure the data returned from an SQL command is a standard tuple or list[tuple].

Expand All @@ -444,6 +436,13 @@ def _make_common_data_structure(self, result: T | Sequence[T]) -> tuple | list[t
# Back-compatibility call for providers implementing old ´_make_serializable' method.
with contextlib.suppress(AttributeError):
result = self._make_serializable(result=result) # type: ignore[attr-defined]
warnings.warn(
"The `_make_serializable` method is deprecated and support will be removed in a future "
f"version of the common.sql provider. Please update the {self.__class__.__name__}'s provider "
"to a version based on common.sql >= 1.9.1.",
AirflowProviderDeprecationWarning,
stacklevel=2,
)

if isinstance(result, Sequence):
return cast(List[tuple], result)
Expand Down
24 changes: 24 additions & 0 deletions tests/providers/common/sql/hooks/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
#
from __future__ import annotations

import warnings
from typing import Any
from unittest.mock import MagicMock

import pytest

from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.models import Connection
from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler
from airflow.utils.session import provide_session
Expand Down Expand Up @@ -226,3 +229,24 @@ def test_no_query(empty_statement):
with pytest.raises(ValueError) as err:
dbapi_hook.run(sql=empty_statement)
assert err.value.args[0] == "List of SQL statements is empty"


@pytest.mark.db_test
def test_make_common_data_structure_hook_has_deprecated_method():
"""If hook implements ``_make_serializable`` warning should be raised on call."""

class DBApiHookForMakeSerializableTests(DBApiHookForTests):
def _make_serializable(self, result: Any):
return result

hook = DBApiHookForMakeSerializableTests()
with pytest.warns(AirflowProviderDeprecationWarning, match="`_make_serializable` method is deprecated"):
hook._make_common_data_structure(["foo", "bar", "baz"])


@pytest.mark.db_test
def test_make_common_data_structure_no_deprecated_method():
"""If hook not implements ``_make_serializable`` there is no warning should be raised on call."""
with warnings.catch_warnings():
warnings.simplefilter("error", AirflowProviderDeprecationWarning)
DBApiHookForTests()._make_common_data_structure(["foo", "bar", "baz"])