Skip to content
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
1 change: 0 additions & 1 deletion providers/slack/src/airflow/providers/slack/hooks/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 17 additions & 8 deletions providers/slack/src/airflow/providers/slack/operators/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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."""
Expand All @@ -122,12 +130,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(
Expand Down Expand Up @@ -162,7 +164,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,
Expand Down
1 change: 0 additions & 1 deletion providers/slack/tests/system/slack/example_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
10 changes: 2 additions & 8 deletions providers/slack/tests/unit/slack/hooks/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
36 changes: 9 additions & 27 deletions providers/slack/tests/unit/slack/operators/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand All @@ -210,74 +208,58 @@ 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,
content="test-content",
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,
)

@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,
channels="C1234567890",
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,
Expand Down
18 changes: 6 additions & 12 deletions providers/slack/tests/unit/slack/transfers/test_sql_to_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -80,13 +84,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"),
],
)
def test_send_file(
self,
mock_slack_hook_cls,
Expand All @@ -99,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()
Expand All @@ -119,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,
}
Expand Down
Loading