Skip to content

Commit

Permalink
Undeprecating DBApiHookForTests._make_common_data_structure (#38573)
Browse files Browse the repository at this point in the history
* Undeprecating `DBApiHookForTests._make_common_data_structure`

* Add missing db_test marker
  • Loading branch information
Taragolis authored Mar 28, 2024
1 parent d4c2ea4 commit b06f401
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
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"])

0 comments on commit b06f401

Please sign in to comment.