From 7ef54dad3687efc04629d0613ea594b9b09a2b4e Mon Sep 17 00:00:00 2001 From: Pavan Kumar Date: Sat, 2 Aug 2025 22:52:07 +0100 Subject: [PATCH 1/4] Remove deprecated method check send_file usage from slack operators --- providers/slack/pyproject.toml | 2 +- .../airflow/providers/slack/hooks/slack.py | 1 - .../providers/slack/operators/slack.py | 25 ++++++++----- .../providers/slack/transfers/sql_to_slack.py | 8 +---- .../tests/unit/slack/hooks/test_slack.py | 10 ++---- .../tests/unit/slack/operators/test_slack.py | 36 +++++-------------- .../unit/slack/transfers/test_sql_to_slack.py | 7 ++-- 7 files changed, 35 insertions(+), 54 deletions(-) diff --git a/providers/slack/pyproject.toml b/providers/slack/pyproject.toml index 253896b6bee91..2501bed2a6fb7 100644 --- a/providers/slack/pyproject.toml +++ b/providers/slack/pyproject.toml @@ -60,7 +60,7 @@ dependencies = [ "apache-airflow>=2.10.0", "apache-airflow-providers-common-compat>=1.6.1", "apache-airflow-providers-common-sql>=1.27.0", - "slack_sdk>=3.19.0", + "slack_sdk>=3.36.0", ] [dependency-groups] diff --git a/providers/slack/src/airflow/providers/slack/hooks/slack.py b/providers/slack/src/airflow/providers/slack/hooks/slack.py index 10538a2b3b664..7ea38a031013f 100644 --- a/providers/slack/src/airflow/providers/slack/hooks/slack.py +++ b/providers/slack/src/airflow/providers/slack/hooks/slack.py @@ -237,7 +237,6 @@ def send_file_v1_to_v2( initial_comment: str | None = None, title: str | None = None, snippet_type: str | None = None, - **kwargs, ) -> list[SlackResponse]: """ Smooth transition between ``send_file`` and ``send_file_v2`` methods. diff --git a/providers/slack/src/airflow/providers/slack/operators/slack.py b/providers/slack/src/airflow/providers/slack/operators/slack.py index 07c9289f14cbb..35f3712ff50dd 100644 --- a/providers/slack/src/airflow/providers/slack/operators/slack.py +++ b/providers/slack/src/airflow/providers/slack/operators/slack.py @@ -18,10 +18,12 @@ from __future__ import annotations import json +import warnings from collections.abc import Sequence from functools import cached_property from typing import TYPE_CHECKING, Any, Literal +from airflow.exceptions import AirflowProviderDeprecationWarning from airflow.providers.slack.hooks.slack import SlackHook from airflow.providers.slack.version_compat import BaseOperator @@ -225,7 +227,7 @@ def __init__( filetype: str | None = None, content: str | None = None, title: str | None = None, - method_version: Literal["v1", "v2"] = "v2", + method_version: Literal["v1", "v2"] | None = None, snippet_type: str | None = None, **kwargs, ) -> None: @@ -239,18 +241,25 @@ def __init__( self.method_version = method_version self.snippet_type = snippet_type - @property - def _method_resolver(self): - if self.method_version == "v1": - return self.hook.send_file - return self.hook.send_file_v1_to_v2 + if self.filetype: + warnings.warn( + "The property `filetype` is no longer supported in slack_sdk and will be removed in a future release.", + AirflowProviderDeprecationWarning, + stacklevel=2, + ) + + if self.method_version: + warnings.warn( + "The property `method_version` is no longer required for `SlackAPIFileOperator`, as slack_sdk is using the files_upload_v2 method by default.", + AirflowProviderDeprecationWarning, + stacklevel=2, + ) def execute(self, context: Context): - self._method_resolver( + self.hook.send_file_v1_to_v2( channels=self.channels, # For historical reason SlackAPIFileOperator use filename as reference to file file=self.filename, - filetype=self.filetype, content=self.content, initial_comment=self.initial_comment, title=self.title, diff --git a/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py b/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py index dfe0f07e6146d..3403c83b7f4e7 100644 --- a/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py +++ b/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py @@ -122,12 +122,6 @@ def slack_hook(self): retry_handlers=self.slack_retry_handlers, ) - @property - def _method_resolver(self): - if self.slack_method_version == "v1": - return self.slack_hook.send_file - return self.slack_hook.send_file_v1_to_v2 - def execute(self, context: Context) -> None: # Parse file format from filename output_file_format, _ = parse_filename( @@ -162,7 +156,7 @@ def execute(self, context: Context) -> None: # if SUPPORTED_FILE_FORMATS extended and no actual implementation for specific format. raise AirflowException(f"Unexpected output file format: {output_file_format}") - self._method_resolver( + self.slack_hook.send_file_v1_to_v2( channels=self.slack_channels, file=output_file_name, filename=self.slack_filename, diff --git a/providers/slack/tests/unit/slack/hooks/test_slack.py b/providers/slack/tests/unit/slack/hooks/test_slack.py index cd7e3321191c4..3ce39f1218552 100644 --- a/providers/slack/tests/unit/slack/hooks/test_slack.py +++ b/providers/slack/tests/unit/slack/hooks/test_slack.py @@ -463,11 +463,8 @@ def test_send_file_v2_channel_name(self, mocked_client, caplog): @pytest.mark.parametrize("title", [None, "test title"]) @pytest.mark.parametrize("filename", [None, "foo.bar"]) @pytest.mark.parametrize("channel", [None, "#random"]) - @pytest.mark.parametrize("filetype", [None, "auto"]) @pytest.mark.parametrize("snippet_type", [None, "text"]) - def test_send_file_v1_to_v2_content( - self, initial_comment, title, filename, channel, filetype, snippet_type - ): + def test_send_file_v1_to_v2_content(self, initial_comment, title, filename, channel, snippet_type): hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) with mock.patch.object(SlackHook, "send_file_v2") as mocked_send_file_v2: hook.send_file_v1_to_v2( @@ -476,7 +473,6 @@ def test_send_file_v1_to_v2_content( filename=filename, initial_comment=initial_comment, title=title, - filetype=filetype, snippet_type=snippet_type, ) mocked_send_file_v2.assert_called_once_with( @@ -494,9 +490,8 @@ def test_send_file_v1_to_v2_content( @pytest.mark.parametrize("title", [None, "test title"]) @pytest.mark.parametrize("filename", [None, "foo.bar"]) @pytest.mark.parametrize("channel", [None, "#random"]) - @pytest.mark.parametrize("filetype", [None, "auto"]) @pytest.mark.parametrize("snippet_type", [None, "text"]) - def test_send_file_v1_to_v2_file(self, initial_comment, title, filename, channel, filetype, snippet_type): + def test_send_file_v1_to_v2_file(self, initial_comment, title, filename, channel, snippet_type): hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) with mock.patch.object(SlackHook, "send_file_v2") as mocked_send_file_v2: hook.send_file_v1_to_v2( @@ -505,7 +500,6 @@ def test_send_file_v1_to_v2_file(self, initial_comment, title, filename, channel filename=filename, initial_comment=initial_comment, title=title, - filetype=filetype, snippet_type=snippet_type, ) mocked_send_file_v2.assert_called_once_with( diff --git a/providers/slack/tests/unit/slack/operators/test_slack.py b/providers/slack/tests/unit/slack/operators/test_slack.py index 4aef638af2ef3..1a17c260817c3 100644 --- a/providers/slack/tests/unit/slack/operators/test_slack.py +++ b/providers/slack/tests/unit/slack/operators/test_slack.py @@ -181,7 +181,6 @@ def setup_method(self): self.test_channel = "#test_slack_channel" self.test_initial_comment = "test text file test_filename.txt" self.filename = "test_filename.txt" - self.test_filetype = "text" self.test_content = "This is a test text file!" self.test_api_params = {"key": "value"} self.expected_method = "files.upload" @@ -194,7 +193,6 @@ def __construct_operator(self, test_slack_conn_id, test_api_params=None): channels=self.test_channel, initial_comment=self.test_initial_comment, filename=self.filename, - filetype=self.test_filetype, content=self.test_content, api_params=test_api_params, snippet_type=self.test_snippet_type, @@ -210,23 +208,15 @@ def test_init_with_valid_params(self): assert slack_api_post_operator.channels == self.test_channel assert slack_api_post_operator.api_params == self.test_api_params assert slack_api_post_operator.filename == self.filename - assert slack_api_post_operator.filetype == self.test_filetype + assert slack_api_post_operator.filetype is None assert slack_api_post_operator.content == self.test_content assert slack_api_post_operator.snippet_type == self.test_snippet_type assert not hasattr(slack_api_post_operator, "token") @pytest.mark.parametrize("initial_comment", [None, "foo-bar"]) @pytest.mark.parametrize("title", [None, "Spam Egg"]) - @pytest.mark.parametrize( - "method_version, method_name", - [ - pytest.param("v2", "send_file_v1_to_v2", id="v2"), - ], - ) @pytest.mark.parametrize("snippet_type", [None, "text"]) - def test_api_call_params_with_content_args( - self, initial_comment, title, method_version, method_name, snippet_type - ): + def test_api_call_params_with_content_args(self, initial_comment, title, snippet_type): op = SlackAPIFileOperator( task_id="slack", slack_conn_id=SLACK_API_TEST_CONNECTION_ID, @@ -234,16 +224,16 @@ def test_api_call_params_with_content_args( channels="#test-channel", initial_comment=initial_comment, title=title, - method_version=method_version, snippet_type=snippet_type, ) - with mock.patch(f"airflow.providers.slack.operators.slack.SlackHook.{method_name}") as mock_send_file: + with mock.patch( + "airflow.providers.slack.operators.slack.SlackHook.send_file_v1_to_v2" + ) as mock_send_file: op.execute({}) mock_send_file.assert_called_once_with( channels="#test-channel", content="test-content", file=None, - filetype=None, initial_comment=initial_comment, title=title, snippet_type=snippet_type, @@ -251,16 +241,8 @@ def test_api_call_params_with_content_args( @pytest.mark.parametrize("initial_comment", [None, "foo-bar"]) @pytest.mark.parametrize("title", [None, "Spam Egg"]) - @pytest.mark.parametrize( - "method_version, method_name", - [ - pytest.param("v2", "send_file_v1_to_v2", id="v2"), - ], - ) @pytest.mark.parametrize("snippet_type", [None, "text"]) - def test_api_call_params_with_file_args( - self, initial_comment, title, method_version, method_name, snippet_type - ): + def test_api_call_params_with_file_args(self, initial_comment, title, snippet_type): op = SlackAPIFileOperator( task_id="slack", slack_conn_id=SLACK_API_TEST_CONNECTION_ID, @@ -268,16 +250,16 @@ def test_api_call_params_with_file_args( filename="/dev/null", initial_comment=initial_comment, title=title, - method_version=method_version, snippet_type=snippet_type, ) - with mock.patch(f"airflow.providers.slack.operators.slack.SlackHook.{method_name}") as mock_send_file: + with mock.patch( + "airflow.providers.slack.operators.slack.SlackHook.send_file_v1_to_v2" + ) as mock_send_file: op.execute({}) mock_send_file.assert_called_once_with( channels="C1234567890", content=None, file="/dev/null", - filetype=None, initial_comment=initial_comment, title=title, snippet_type=snippet_type, diff --git a/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py b/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py index cdf6ee41122b2..c45e347e5adba 100644 --- a/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py +++ b/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py @@ -22,7 +22,11 @@ from airflow.exceptions import AirflowSkipException from airflow.providers.slack.transfers.sql_to_slack import SqlToSlackApiFileOperator -from airflow.utils import timezone + +try: + from airflow.sdk import timezone +except ImportError: + from airflow.utils import timezone # type: ignore[attr-defined,no-redef] TEST_DAG_ID = "sql_to_slack_unit_test" TEST_TASK_ID = "sql_to_slack_unit_test_task" @@ -83,7 +87,6 @@ def setup_method(self): @pytest.mark.parametrize( "method_version, method_name", [ - pytest.param("v1", "send_file", id="v1"), pytest.param("v2", "send_file_v1_to_v2", id="v2"), ], ) From a623b36d728639f8653c0c5423fd92ce6f4b2328 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Date: Sat, 2 Aug 2025 23:39:33 +0100 Subject: [PATCH 2/4] Fix tests --- providers/slack/tests/system/slack/example_slack.py | 1 - 1 file changed, 1 deletion(-) diff --git a/providers/slack/tests/system/slack/example_slack.py b/providers/slack/tests/system/slack/example_slack.py index aaa72767657c5..1dad907eb0961 100644 --- a/providers/slack/tests/system/slack/example_slack.py +++ b/providers/slack/tests/system/slack/example_slack.py @@ -76,7 +76,6 @@ task_id="slack_file_upload_1", channels=SLACK_CHANNEL, filename="/files/dags/test.txt", - filetype="txt", ) # [END slack_api_file_operator_howto_guide] From 9586e1dcc4cfcc3c630c561a9f4f593404032314 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Date: Sun, 3 Aug 2025 09:31:16 +0100 Subject: [PATCH 3/4] Update warning --- .../providers/slack/transfers/sql_to_slack.py | 12 ++++++++++-- .../tests/unit/slack/transfers/test_sql_to_slack.py | 11 +---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py b/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py index 3403c83b7f4e7..176ddccfaa2e4 100644 --- a/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py +++ b/providers/slack/src/airflow/providers/slack/transfers/sql_to_slack.py @@ -16,12 +16,13 @@ # under the License. from __future__ import annotations +import warnings from collections.abc import Mapping, Sequence from functools import cached_property from tempfile import NamedTemporaryFile from typing import TYPE_CHECKING, Any, Literal -from airflow.exceptions import AirflowException, AirflowSkipException +from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning, AirflowSkipException from airflow.providers.slack.hooks.slack import SlackHook from airflow.providers.slack.transfers.base_sql_to_slack import BaseSqlToSlackOperator from airflow.providers.slack.utils import parse_filename @@ -91,7 +92,7 @@ def __init__( slack_initial_comment: str | None = None, slack_title: str | None = None, slack_base_url: str | None = None, - slack_method_version: Literal["v1", "v2"] = "v2", + slack_method_version: Literal["v1", "v2"] | None = None, df_kwargs: dict | None = None, action_on_empty_df: Literal["send", "skip", "error"] = "send", **kwargs, @@ -111,6 +112,13 @@ def __init__( raise ValueError(f"Invalid `action_on_empty_df` value {action_on_empty_df!r}") self.action_on_empty_df = action_on_empty_df + if self.slack_method_version: + warnings.warn( + "The property `slack_method_version` is no longer required for `SqlToSlackApiFileOperator`, as slack_sdk is using the files_upload_v2 method by default.", + AirflowProviderDeprecationWarning, + stacklevel=2, + ) + @cached_property def slack_hook(self): """Slack API Hook.""" diff --git a/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py b/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py index c45e347e5adba..a6daaca07dd28 100644 --- a/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py +++ b/providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py @@ -84,12 +84,6 @@ def setup_method(self): ), ], ) - @pytest.mark.parametrize( - "method_version, method_name", - [ - pytest.param("v2", "send_file_v1_to_v2", id="v2"), - ], - ) def test_send_file( self, mock_slack_hook_cls, @@ -102,12 +96,10 @@ def test_send_file( title, slack_op_kwargs: dict, hook_extra_kwargs: dict, - method_version, - method_name: str, ): # Mock Hook mock_send_file = mock.MagicMock() - setattr(mock_slack_hook_cls.return_value, method_name, mock_send_file) + setattr(mock_slack_hook_cls.return_value, "send_file_v1_to_v2", mock_send_file) # Mock returns pandas.DataFrame and expected method mock_df = mock.MagicMock() @@ -122,7 +114,6 @@ def test_send_file( "slack_channels": channels, "slack_initial_comment": initial_comment, "slack_title": title, - "slack_method_version": method_version, "df_kwargs": df_kwargs, **slack_op_kwargs, } From af6518f3c75e864b28893a8d900e370f1d4127b3 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Date: Mon, 4 Aug 2025 08:11:00 +0100 Subject: [PATCH 4/4] Remove slack_sdk version bump --- providers/slack/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/slack/pyproject.toml b/providers/slack/pyproject.toml index 2501bed2a6fb7..253896b6bee91 100644 --- a/providers/slack/pyproject.toml +++ b/providers/slack/pyproject.toml @@ -60,7 +60,7 @@ dependencies = [ "apache-airflow>=2.10.0", "apache-airflow-providers-common-compat>=1.6.1", "apache-airflow-providers-common-sql>=1.27.0", - "slack_sdk>=3.36.0", + "slack_sdk>=3.19.0", ] [dependency-groups]