This is an automated email from the ASF dual-hosted git repository.
eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 9758acf6c2 Optionally use `client.files_upload_v2` in Slack Provider
(#36757)
9758acf6c2 is described below
commit 9758acf6c2ca100fa7fc2e4f324a95d2a7189e97
Author: Andrey Anshin <[email protected]>
AuthorDate: Sat Jan 20 19:41:50 2024 +0300
Optionally use `client.files_upload_v2` in Slack Provider (#36757)
* Optionally use `client.files_upload_v2` in Slack Provider
* Revert default value for method
* Remove redundand assigment
---
airflow/providers/slack/hooks/slack.py | 148 ++++++++-
airflow/providers/slack/operators/slack.py | 40 ++-
airflow/providers/slack/provider.yaml | 2 +-
airflow/providers/slack/transfers/sql_to_slack.py | 32 +-
.../operators/slack_api.rst | 18 +-
.../operators/sql_to_slack.rst | 18 ++
generated/provider_dependencies.json | 2 +-
pyproject.toml | 2 +-
tests/providers/slack/hooks/test_slack.py | 332 +++++++++++++++------
tests/providers/slack/operators/test_slack.py | 82 +++--
.../providers/slack/transfers/test_sql_to_slack.py | 14 +-
tests/system/providers/slack/example_slack.py | 1 +
12 files changed, 547 insertions(+), 144 deletions(-)
diff --git a/airflow/providers/slack/hooks/slack.py
b/airflow/providers/slack/hooks/slack.py
index 50abcb63f5..b25282f370 100644
--- a/airflow/providers/slack/hooks/slack.py
+++ b/airflow/providers/slack/hooks/slack.py
@@ -21,10 +21,11 @@ import json
import warnings
from functools import cached_property
from pathlib import Path
-from typing import TYPE_CHECKING, Any, Sequence
+from typing import TYPE_CHECKING, Any, Sequence, TypedDict
from slack_sdk import WebClient
from slack_sdk.errors import SlackApiError
+from typing_extensions import NotRequired
from airflow.exceptions import AirflowNotFoundException
from airflow.hooks.base import BaseHook
@@ -36,6 +37,26 @@ if TYPE_CHECKING:
from slack_sdk.web.slack_response import SlackResponse
+class FileUploadTypeDef(TypedDict):
+ """
+ Represents the structure of the file upload data.
+
+ :ivar file: Optional. Path to file which need to be sent.
+ :ivar content: Optional. File contents. If omitting this parameter, you
must provide a file.
+ :ivar filename: Optional. Displayed filename.
+ :ivar title: Optional. The title of the uploaded file.
+ :ivar alt_txt: Optional. Description of image for screen-reader.
+ :ivar snippet_type: Optional. Syntax type of the snippet being uploaded.
+ """
+
+ file: NotRequired[str | None]
+ content: NotRequired[str | None]
+ filename: NotRequired[str | None]
+ title: NotRequired[str | None]
+ alt_txt: NotRequired[str | None]
+ snippet_type: NotRequired[str | None]
+
+
class SlackHook(BaseHook):
"""
Creates a Slack API Connection to be used for calls.
@@ -111,6 +132,9 @@ class SlackHook(BaseHook):
extra_client_args["logger"] = self.log
self.extra_client_args = extra_client_args
+ # Use for caching channels result
+ self._channels_mapping: dict[str, str] = {}
+
@cached_property
def client(self) -> WebClient:
"""Get the underlying slack_sdk.WebClient (cached)."""
@@ -212,6 +236,128 @@ class SlackHook(BaseHook):
channels=channels,
)
+ def send_file_v2(
+ self,
+ *,
+ channel_id: str | None = None,
+ file_uploads: FileUploadTypeDef | list[FileUploadTypeDef],
+ initial_comment: str | None = None,
+ ) -> SlackResponse:
+ """
+ Sends one or more files to a Slack channel using the Slack SDK Client
method `files_upload_v2`.
+
+ :param channel_id: The ID of the channel to send the file to.
+ If omitting this parameter, then file will send to workspace.
+ :param file_uploads: The file(s) specification to upload.
+ :param initial_comment: The message text introducing the file in
specified ``channel``.
+ """
+ if channel_id and channel_id.startswith("#"):
+ retried_channel_id = self.get_channel_id(channel_id[1:])
+ warnings.warn(
+ "The method `files_upload_v2` in the Slack SDK Client expects
a Slack Channel ID, "
+ f"but received a Slack Channel Name. To resolve this, consider
replacing {channel_id!r} "
+ f"with the corresponding Channel ID {retried_channel_id!r}.",
+ UserWarning,
+ stacklevel=2,
+ )
+ channel_id = retried_channel_id
+
+ if not isinstance(file_uploads, list):
+ file_uploads = [file_uploads]
+ for file_upload in file_uploads:
+ if not file_upload.get("filename"):
+ # Some of early version of Slack SDK (such as 3.19.0) raise an
error if ``filename`` not set.
+ file_upload["filename"] = "Uploaded file"
+
+ return self.client.files_upload_v2(
+ channel=channel_id,
+ # mypy doesn't happy even if TypedDict used instead of dict[str,
Any]
+ # see: https://github.com/python/mypy/issues/4976
+ file_uploads=file_uploads, # type: ignore[arg-type]
+ initial_comment=initial_comment,
+ )
+
+ def send_file_v1_to_v2(
+ self,
+ *,
+ channels: str | Sequence[str] | None = None,
+ file: str | Path | None = None,
+ content: str | None = None,
+ filename: str | None = None,
+ initial_comment: str | None = None,
+ title: str | None = None,
+ filetype: str | None = None,
+ ) -> list[SlackResponse]:
+ """
+ Smooth transition between ``send_file`` and ``send_file_v2`` methods.
+
+ :param channels: Comma-separated list of channel names or IDs where
the file will be shared.
+ If omitting this parameter, then file will send to workspace.
+ File would be uploaded for each channel individually.
+ :param file: Path to file which need to be sent.
+ :param content: File contents. If omitting this parameter, you must
provide a file.
+ :param filename: Displayed filename.
+ :param initial_comment: The message text introducing the file in
specified ``channels``.
+ :param title: Title of the file.
+ :param filetype: A file type identifier.
+ """
+ if not exactly_one(file, content):
+ raise ValueError("Either `file` or `content` must be provided, not
both.")
+ if file:
+ file = Path(file)
+ file_uploads: FileUploadTypeDef = {"file": file.__fspath__(),
"filename": filename or file.name}
+ else:
+ file_uploads = {"content": content, "filename": filename}
+
+ file_uploads.update({"title": title, "snippet_type": filetype})
+
+ if channels:
+ if isinstance(channels, str):
+ channels = channels.split(",")
+ channels_to_share: list[str | None] = list(map(str.strip,
channels))
+ else:
+ channels_to_share = [None]
+
+ responses = []
+ for channel in channels_to_share:
+ responses.append(
+ self.send_file_v2(
+ channel_id=channel, file_uploads=file_uploads,
initial_comment=initial_comment
+ )
+ )
+ return responses
+
+ def get_channel_id(self, channel_name: str) -> str:
+ """
+ Retrieves a Slack channel id by a channel name.
+
+ It continuously iterates over all Slack channels (public and private)
+ until it finds the desired channel name in addition cache results for
further usage.
+
+ .. seealso::
+ https://api.slack.com/methods/conversations.list
+
+ :param channel_name: The name of the Slack channel for which ID has to
be found.
+ """
+ next_cursor = None
+ while not (channel_id := self._channels_mapping.get(channel_name)):
+ res = self.client.conversations_list(cursor=next_cursor,
types="public_channel,private_channel")
+ if TYPE_CHECKING:
+ # Slack SDK response type too broad, this should make mypy
happy
+ assert isinstance(res.data, dict)
+
+ for channel_data in res.data.get("channels", []):
+ self._channels_mapping[channel_data["name"]] =
channel_data["id"]
+
+ if not (next_cursor := res.data.get("response_metadata",
{}).get("next_cursor")):
+ channel_id = self._channels_mapping.get(channel_name)
+ break
+
+ if not channel_id:
+ msg = f"Unable to find slack channel with name: {channel_name!r}"
+ raise LookupError(msg)
+ return channel_id
+
def test_connection(self):
"""Tests the Slack API connection.
diff --git a/airflow/providers/slack/operators/slack.py
b/airflow/providers/slack/operators/slack.py
index 4062eb6d55..838786f332 100644
--- a/airflow/providers/slack/operators/slack.py
+++ b/airflow/providers/slack/operators/slack.py
@@ -22,6 +22,8 @@ import warnings
from functools import cached_property
from typing import TYPE_CHECKING, Any, Sequence
+from typing_extensions import Literal
+
from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.models import BaseOperator
from airflow.providers.slack.hooks.slack import SlackHook
@@ -29,13 +31,15 @@ from airflow.providers.slack.hooks.slack import SlackHook
if TYPE_CHECKING:
from slack_sdk.http_retry import RetryHandler
+ from airflow.utils.context import Context
+
class SlackAPIOperator(BaseOperator):
"""Base Slack Operator class.
:param slack_conn_id: :ref:`Slack API Connection <howto/connection:slack>`
which its password is Slack API token.
- :param method: The Slack API Method to Call
(https://api.slack.com/methods). Optional
+ :param method: The Slack API Method to Call
(https://api.slack.com/methods).
:param api_params: API Method call parameters
(https://api.slack.com/methods). Optional
:param timeout: The maximum number of seconds the client will wait to
connect
and receive a response from Slack. Optional
@@ -56,6 +60,13 @@ class SlackAPIOperator(BaseOperator):
retry_handlers: list[RetryHandler] | None = None,
**kwargs,
) -> None:
+ if not method:
+ warnings.warn(
+ "Define `method` parameter as empty string or None is
deprecated. "
+ "In the future it will raise an error on task initialisation.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
super().__init__(**kwargs)
self.slack_conn_id = slack_conn_id
self.method = method
@@ -90,7 +101,10 @@ class SlackAPIOperator(BaseOperator):
"SlackAPIOperator should not be used directly. Chose one of the
subclasses instead"
)
- def execute(self, **kwargs):
+ def execute(self, context: Context):
+ if not self.method:
+ msg = f"Expected non empty `method` attribute in
{type(self).__name__!r}, but got {self.method!r}"
+ raise ValueError(msg)
if not self.api_params:
self.construct_api_call_params()
self.hook.call(self.method, json=self.api_params)
@@ -138,14 +152,13 @@ class SlackAPIPostOperator(SlackAPIOperator):
attachments: list | None = None,
**kwargs,
) -> None:
- self.method = "chat.postMessage"
+ super().__init__(method="chat.postMessage", **kwargs)
self.channel = channel
self.username = username
self.text = text
self.icon_url = icon_url
self.attachments = attachments or []
self.blocks = blocks or []
- super().__init__(method=self.method, **kwargs)
def construct_api_call_params(self) -> Any:
self.api_params = {
@@ -191,7 +204,7 @@ class SlackAPIFileOperator(SlackAPIOperator):
:param filetype: slack filetype. (templated) See:
https://api.slack.com/types/file#file_types
:param content: file content. (templated)
:param title: title of file. (templated)
- :param channel: (deprecated) channel in which to sent file on slack name
+ :param method_version: The version of the method of Slack SDK Client to be
used, either "v1" or "v2".
"""
template_fields: Sequence[str] = (
@@ -212,10 +225,10 @@ class SlackAPIFileOperator(SlackAPIOperator):
filetype: str | None = None,
content: str | None = None,
title: str | None = None,
- channel: str | None = None,
+ method_version: Literal["v1", "v2"] = "v1",
**kwargs,
) -> None:
- if channel:
+ if channel := kwargs.pop("channel", None):
warnings.warn(
"Argument `channel` is deprecated and will removed in a future
releases. "
"Please use `channels` instead.",
@@ -226,16 +239,23 @@ class SlackAPIFileOperator(SlackAPIOperator):
raise ValueError(f"Cannot set both arguments:
channel={channel!r} and channels={channels!r}.")
channels = channel
+ super().__init__(method="files.upload", **kwargs)
self.channels = channels
self.initial_comment = initial_comment
self.filename = filename
self.filetype = filetype
self.content = content
self.title = title
- super().__init__(method="files.upload", **kwargs)
+ self.method_version = method_version
+
+ @property
+ def _method_resolver(self):
+ if self.method_version == "v1":
+ return self.hook.send_file
+ return self.hook.send_file_v1_to_v2
- def execute(self, **kwargs):
- self.hook.send_file(
+ def execute(self, context: Context):
+ self._method_resolver(
channels=self.channels,
# For historical reason SlackAPIFileOperator use filename as
reference to file
file=self.filename,
diff --git a/airflow/providers/slack/provider.yaml
b/airflow/providers/slack/provider.yaml
index ede4202c6b..d8c95c4ab4 100644
--- a/airflow/providers/slack/provider.yaml
+++ b/airflow/providers/slack/provider.yaml
@@ -58,7 +58,7 @@ versions:
dependencies:
- apache-airflow>=2.6.0
- apache-airflow-providers-common-sql>=1.3.1
- - slack_sdk>=3.0.0
+ - slack_sdk>=3.19.0
integrations:
- integration-name: Slack
diff --git a/airflow/providers/slack/transfers/sql_to_slack.py
b/airflow/providers/slack/transfers/sql_to_slack.py
index 6f2ab24f85..1639798c24 100644
--- a/airflow/providers/slack/transfers/sql_to_slack.py
+++ b/airflow/providers/slack/transfers/sql_to_slack.py
@@ -17,9 +17,12 @@
from __future__ import annotations
import warnings
+from functools import cached_property
from tempfile import NamedTemporaryFile
from typing import TYPE_CHECKING, Any, Mapping, Sequence
+from typing_extensions import Literal
+
from airflow.exceptions import AirflowException,
AirflowProviderDeprecationWarning
from airflow.providers.slack.hooks.slack import SlackHook
from airflow.providers.slack.transfers.base_sql_to_slack import
BaseSqlToSlackOperator
@@ -53,6 +56,7 @@ class SqlToSlackApiFileOperator(BaseSqlToSlackOperator):
:param slack_initial_comment: The message text introducing the file in
specified ``slack_channels``.
:param slack_title: Title of file.
:param slack_base_url: A string representing the Slack API base URL.
Optional
+ :param slack_method_version: The version of the Slack SDK Client method to
be used, either "v1" or "v2".
:param df_kwargs: Keyword arguments forwarded to
``pandas.DataFrame.to_{format}()`` method.
"""
@@ -81,6 +85,7 @@ class SqlToSlackApiFileOperator(BaseSqlToSlackOperator):
slack_initial_comment: str | None = None,
slack_title: str | None = None,
slack_base_url: str | None = None,
+ slack_method_version: Literal["v1", "v2"] = "v1",
df_kwargs: dict | None = None,
**kwargs,
):
@@ -93,8 +98,26 @@ class SqlToSlackApiFileOperator(BaseSqlToSlackOperator):
self.slack_initial_comment = slack_initial_comment
self.slack_title = slack_title
self.slack_base_url = slack_base_url
+ self.slack_method_version = slack_method_version
self.df_kwargs = df_kwargs or {}
+ @cached_property
+ def slack_hook(self):
+ """Slack API Hook."""
+ return SlackHook(
+ slack_conn_id=self.slack_conn_id,
+ base_url=self.slack_base_url,
+ timeout=self.slack_timeout,
+ proxy=self.slack_proxy,
+ 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(
@@ -102,13 +125,6 @@ class SqlToSlackApiFileOperator(BaseSqlToSlackOperator):
supported_file_formats=self.SUPPORTED_FILE_FORMATS,
)
- slack_hook = SlackHook(
- slack_conn_id=self.slack_conn_id,
- base_url=self.slack_base_url,
- timeout=self.slack_timeout,
- proxy=self.slack_proxy,
- retry_handlers=self.slack_retry_handlers,
- )
with NamedTemporaryFile(mode="w+", suffix=f"_{self.slack_filename}")
as fp:
# tempfile.NamedTemporaryFile used only for create and remove
temporary file,
# pandas will open file in correct mode itself depend on file type.
@@ -129,7 +145,7 @@ class SqlToSlackApiFileOperator(BaseSqlToSlackOperator):
# if SUPPORTED_FILE_FORMATS extended and no actual
implementation for specific format.
raise AirflowException(f"Unexpected output file format:
{output_file_format}")
- slack_hook.send_file(
+ self._method_resolver(
channels=self.slack_channels,
file=output_file_name,
filename=self.slack_filename,
diff --git a/docs/apache-airflow-providers-slack/operators/slack_api.rst
b/docs/apache-airflow-providers-slack/operators/slack_api.rst
index c8af3ab703..6b2debb405 100644
--- a/docs/apache-airflow-providers-slack/operators/slack_api.rst
+++ b/docs/apache-airflow-providers-slack/operators/slack_api.rst
@@ -55,10 +55,26 @@ SlackAPIFileOperator
Use the :class:`~airflow.providers.slack.operators.slack.SlackAPIFileOperator`
to send files to a Slack channel(s).
-
Using the Operator
^^^^^^^^^^^^^^^^^^
+.. note::
+ Operator supports two methods for upload files, which controlled by
``method_version``,
+ by default it use Slack SDK method ``upload_files`` however this might
impact a performance and cause random API errors.
+ It is recommended to switch to Slack SDK method ``upload_files_v2`` by set
``v2`` to ``method_version``,
+ however this action required to add additional scope to your application:
+
+ * **files:write** - for write files.
+ * **files:read** - for read files (not required if you use Slack SDK >=
3.23.0).
+ * **channels:read** - get list of public channels, for convert Channel
Name to Channel ID.
+ * **groups:read** - get list of private channels, for convert Channel Name
to Channel ID
+ * **mpim:read** - additional permission for API method
**conversations.list**
+ * **im:read** - additional permission for API method **conversations.list**
+
+ .. seealso::
+ - `Slack SDK 3.19.0 Release Notes
<https://github.com/slackapi/python-slack-sdk/releases/tag/v3.19.0>`_
+ - `conversations.list API
<https://api.slack.com/methods/conversations.list>`_
+
You could send file attachment by specifying file path
.. exampleinclude:: /../../tests/system/providers/slack/example_slack.py
diff --git a/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst
b/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst
index de9fe136d9..2bf053f5a8 100644
--- a/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst
+++ b/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst
@@ -26,6 +26,24 @@ to Slack channel(s) through `Slack API
<https://api.slack.com/>`__.
Using the Operator
^^^^^^^^^^^^^^^^^^
+.. note::
+ Operator supports two methods for upload files, which controlled by
``slack_method_version``,
+ by default it use Slack SDK method ``upload_files`` however this might
impact a performance and cause random API errors.
+ It is recommended to switch to Slack SDK method ``upload_files_v2`` by set
``v2`` to ``slack_method_version``,
+ however this action required to add additional scope to your application:
+
+ * **files:write** - for write files.
+ * **files:read** - for read files (not required if you use Slack SDK >=
3.23.0).
+ * **channels:read** - get list of public channels, for convert Channel
Name to Channel ID.
+ * **groups:read** - get list of private channels, for convert Channel Name
to Channel ID
+ * **mpim:read** - additional permission for API method
**conversations.list**
+ * **im:read** - additional permission for API method **conversations.list**
+
+ .. seealso::
+ - `Slack SDK 3.19.0 Release Notes
<https://github.com/slackapi/python-slack-sdk/releases/tag/v3.19.0>`_
+ - `conversations.list API
<https://api.slack.com/methods/conversations.list>`_
+
+
This operator will execute a custom query in the provided SQL connection and
publish a file to Slack channel(s).
An example usage of the SqlToSlackApiFileOperator is as follows:
diff --git a/generated/provider_dependencies.json
b/generated/provider_dependencies.json
index cdff33de1a..4d369c2186 100644
--- a/generated/provider_dependencies.json
+++ b/generated/provider_dependencies.json
@@ -1017,7 +1017,7 @@
"deps": [
"apache-airflow-providers-common-sql>=1.3.1",
"apache-airflow>=2.6.0",
- "slack_sdk>=3.0.0"
+ "slack_sdk>=3.19.0"
],
"devel-deps": [],
"cross-providers-deps": [
diff --git a/pyproject.toml b/pyproject.toml
index 28bb06d2ea..43cb43107a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -904,7 +904,7 @@ singularity = [
]
slack = [
"apache-airflow[common_sql]",
- "slack_sdk>=3.0.0",
+ "slack_sdk>=3.19.0",
]
smtp = [
]
diff --git a/tests/providers/slack/hooks/test_slack.py
b/tests/providers/slack/hooks/test_slack.py
index 5dba70fbdd..023c6a9ebd 100644
--- a/tests/providers/slack/hooks/test_slack.py
+++ b/tests/providers/slack/hooks/test_slack.py
@@ -18,10 +18,8 @@
from __future__ import annotations
import json
-import os
from typing import Any
from unittest import mock
-from unittest.mock import patch
import pytest
from slack_sdk.errors import SlackApiError
@@ -73,7 +71,23 @@ def slack_api_connections():
yield
[email protected]
+def mocked_client():
+ with mock.patch.object(SlackHook, "client") as m:
+ yield m
+
+
class TestSlackHook:
+ @staticmethod
+ def fake_slack_response(*, data: dict | bytes, status_code: int = 200,
**kwargs):
+ """Helper for generate fake slack response."""
+ # Mock other mandatory ``SlackResponse`` arguments
+ for mandatory_param in ("client", "http_verb", "api_url", "req_args",
"headers"):
+ if mandatory_param not in kwargs:
+ kwargs[mandatory_param] =
mock.MagicMock(name=f"fake-{mandatory_param}")
+
+ return SlackResponse(status_code=status_code, data=data, **kwargs)
+
@pytest.mark.parametrize(
"conn_id",
[
@@ -217,12 +231,8 @@ class TestSlackHook:
assert hook.get_conn() is client # cached
mock_webclient_cls.assert_called_once_with(**expected)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
- def test_call_with_failure(self, slack_client_class_mock):
- slack_client_mock = mock.Mock()
- slack_client_class_mock.return_value = slack_client_mock
- expected_exception = SlackApiError(message="foo", response="bar")
- slack_client_mock.api_call = mock.Mock(side_effect=expected_exception)
+ def test_call_with_failure(self, mocked_client):
+ mocked_client.api_call.side_effect = SlackApiError(message="foo",
response="bar")
slack_hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
test_method = "test_method"
@@ -231,17 +241,14 @@ class TestSlackHook:
with pytest.raises(SlackApiError):
slack_hook.call(test_method, data=test_api_params)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
- def test_api_call(self, slack_client_class_mock):
- slack_client_mock = mock.Mock()
- slack_client_class_mock.return_value = slack_client_mock
- slack_client_mock.api_call.return_value = {"ok": True}
+ def test_api_call(self, mocked_client):
+ mocked_client.api_call.return_value = {"ok": True}
slack_hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
test_api_json = {"channel": "test_channel"}
slack_hook.call("chat.postMessage", json=test_api_json)
- slack_client_mock.api_call.assert_called_with("chat.postMessage",
json=test_api_json)
+ mocked_client.api_call.assert_called_with("chat.postMessage",
json=test_api_json)
@pytest.mark.parametrize(
"response_data",
@@ -266,20 +273,12 @@ class TestSlackHook:
b"some-binary-data",
],
)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
- def test_hook_connection_success(self, mock_webclient_cls, response_data):
+ def test_hook_connection_success(self, mocked_client, response_data):
"""Test SlackHook success connection."""
- mock_webclient = mock_webclient_cls.return_value
- mock_webclient_call = mock_webclient.api_call
- mock_webclient_call.return_value = SlackResponse(
- status_code=200,
- data=response_data,
- # Mock other mandatory SlackResponse arguments
- **{ma: mock.MagicMock for ma in ("client", "http_verb", "api_url",
"req_args", "headers")},
- )
+ mocked_client.api_call.return_value =
self.fake_slack_response(status_code=200, data=response_data)
hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
conn_test = hook.test_connection()
- mock_webclient_call.assert_called_once_with("auth.test")
+ mocked_client.api_call.assert_called_once_with("auth.test")
assert conn_test[0]
@pytest.mark.parametrize(
@@ -290,41 +289,36 @@ class TestSlackHook:
b"some-binary-data",
],
)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
- def test_hook_connection_failed(self, mock_webclient_cls, response_data):
+ def test_hook_connection_failed(self, mocked_client, response_data):
"""Test SlackHook failure connection."""
- mock_webclient = mock_webclient_cls.return_value
- mock_webclient_call = mock_webclient.api_call
- mock_webclient_call.return_value = SlackResponse(
- status_code=401,
- data=response_data,
- # Mock other mandatory SlackResponse arguments
- **{ma: mock.MagicMock for ma in ("client", "http_verb", "api_url",
"req_args", "headers")},
- )
+ mocked_client.api_call.return_value =
self.fake_slack_response(status_code=401, data=response_data)
hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
conn_test = hook.test_connection()
- mock_webclient_call.assert_called_once_with("auth.test")
+ mocked_client.api_call.assert_called_once_with("auth.test")
assert not conn_test[0]
- @pytest.mark.parametrize("file,content", [(None, None), ("", ""),
("foo.bar", "test-content")])
+ @pytest.mark.parametrize(
+ "file,content",
+ [
+ pytest.param(None, None, id="both-none"),
+ pytest.param("", "", id="both-empty"),
+ pytest.param("foo.bar", "test-content", id="both-specified"),
+ ],
+ )
def test_send_file_wrong_parameters(self, file, content):
hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
error_message = r"Either `file` or `content` must be provided, not
both\."
with pytest.raises(ValueError, match=error_message):
hook.send_file(file=file, content=content)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
@pytest.mark.parametrize("initial_comment", [None, "test comment"])
@pytest.mark.parametrize("title", [None, "test title"])
@pytest.mark.parametrize("filetype", [None, "auto"])
@pytest.mark.parametrize("channels", [None, "#random", "#random,#general",
("#random", "#general")])
def test_send_file_path(
- self, mock_webclient_cls, tmp_path_factory, initial_comment, title,
filetype, channels
+ self, mocked_client, tmp_path_factory, initial_comment, title,
filetype, channels
):
"""Test send file by providing filepath."""
- mock_files_upload = mock.MagicMock()
- mock_webclient_cls.return_value.files_upload = mock_files_upload
-
tmp = tmp_path_factory.mktemp("test_send_file_path")
file = tmp / "test.json"
file.write_text('{"foo": "bar"}')
@@ -339,7 +333,7 @@ class TestSlackHook:
filetype=filetype,
)
- mock_files_upload.assert_called_once_with(
+ mocked_client.files_upload.assert_called_once_with(
channels=channels,
file=mock.ANY, # Validate file properties later
filename="filename.mock",
@@ -349,17 +343,13 @@ class TestSlackHook:
)
# Validate file properties
- mock_file = mock_files_upload.call_args.kwargs["file"]
+ mock_file = mocked_client.files_upload.call_args.kwargs["file"]
assert mock_file.mode == "rb"
assert mock_file.name == str(file)
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
@pytest.mark.parametrize("filename", ["test.json", "1.parquet.snappy"])
- def test_send_file_path_set_filename(self, mock_webclient_cls,
tmp_path_factory, filename):
+ def test_send_file_path_set_filename(self, mocked_client,
tmp_path_factory, filename):
"""Test set filename in send_file method if it not set."""
- mock_files_upload = mock.MagicMock()
- mock_webclient_cls.return_value.files_upload = mock_files_upload
-
tmp = tmp_path_factory.mktemp("test_send_file_path_set_filename")
file = tmp / filename
file.write_text('{"foo": "bar"}')
@@ -367,23 +357,18 @@ class TestSlackHook:
hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
hook.send_file(file=file)
- assert mock_files_upload.call_count == 1
- call_args = mock_files_upload.call_args.kwargs
+ assert mocked_client.files_upload.call_count == 1
+ call_args = mocked_client.files_upload.call_args.kwargs
assert "filename" in call_args
assert call_args["filename"] == filename
- @mock.patch("airflow.providers.slack.hooks.slack.WebClient")
@pytest.mark.parametrize("initial_comment", [None, "test comment"])
@pytest.mark.parametrize("title", [None, "test title"])
@pytest.mark.parametrize("filetype", [None, "auto"])
@pytest.mark.parametrize("filename", [None, "foo.bar"])
@pytest.mark.parametrize("channels", [None, "#random", "#random,#general",
("#random", "#general")])
- def test_send_file_content(
- self, mock_webclient_cls, initial_comment, title, filetype, channels,
filename
- ):
+ def test_send_file_content(self, mocked_client, initial_comment, title,
filetype, channels, filename):
"""Test send file by providing content."""
- mock_files_upload = mock.MagicMock()
- mock_webclient_cls.return_value.files_upload = mock_files_upload
hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
hook.send_file(
channels=channels,
@@ -393,7 +378,7 @@ class TestSlackHook:
title=title,
filetype=filetype,
)
- mock_files_upload.assert_called_once_with(
+ mocked_client.files_upload.assert_called_once_with(
channels=channels,
content='{"foo": "bar"}',
filename=filename,
@@ -414,47 +399,204 @@ class TestSlackHook:
pytest.param("a://:abc@?timeout=123&base_url=base_url&proxy=proxy",
id="no-prefix"),
],
)
- def test_backcompat_prefix_works(self, uri):
- with patch.dict(os.environ, AIRFLOW_CONN_MY_CONN=uri):
- hook = SlackHook(slack_conn_id="my_conn")
+ def test_backcompat_prefix_works(self, uri, monkeypatch):
+ monkeypatch.setenv("AIRFLOW_CONN_MY_CONN", uri)
+ hook = SlackHook(slack_conn_id="my_conn")
+ params = hook._get_conn_params()
+ assert params["token"] == "abc"
+ assert params["timeout"] == 123
+ assert params["base_url"] == "base_url"
+ assert params["proxy"] == "proxy"
+
+ def test_backcompat_prefix_both_causes_warning(self, monkeypatch):
+ monkeypatch.setenv("AIRFLOW_CONN_MY_CONN",
"a://:abc@?extra__slack__timeout=111&timeout=222")
+ hook = SlackHook(slack_conn_id="my_conn")
+ with pytest.warns(Warning, match="Using value for `timeout`"):
params = hook._get_conn_params()
- assert params["token"] == "abc"
- assert params["timeout"] == 123
- assert params["base_url"] == "base_url"
- assert params["proxy"] == "proxy"
-
- def test_backcompat_prefix_both_causes_warning(self):
- with patch.dict(
- in_dict=os.environ,
-
AIRFLOW_CONN_MY_CONN="a://:abc@?extra__slack__timeout=111&timeout=222",
- ):
- hook = SlackHook(slack_conn_id="my_conn")
- with pytest.warns(Warning, match="Using value for `timeout`"):
- params = hook._get_conn_params()
- assert params["timeout"] == 222
-
- def test_empty_string_ignored_prefixed(self):
- with patch.dict(
- in_dict=os.environ,
- AIRFLOW_CONN_MY_CONN=json.dumps(
+ assert params["timeout"] == 222
+
+ def test_empty_string_ignored_prefixed(self, monkeypatch):
+ monkeypatch.setenv(
+ "AIRFLOW_CONN_MY_CONN",
+ json.dumps(
{"password": "hi", "extra": {"extra__slack__base_url": "",
"extra__slack__proxy": ""}}
),
- ):
- hook = SlackHook(slack_conn_id="my_conn")
- params = hook._get_conn_params()
- assert "proxy" not in params
- assert "base_url" not in params
-
- def test_empty_string_ignored_non_prefixed(self):
- with patch.dict(
- in_dict=os.environ,
- AIRFLOW_CONN_MY_CONN=json.dumps({"password": "hi", "extra":
{"base_url": "", "proxy": ""}}),
- ):
- hook = SlackHook(slack_conn_id="my_conn")
- params = hook._get_conn_params()
- assert "proxy" not in params
- assert "base_url" not in params
+ )
+ hook = SlackHook(slack_conn_id="my_conn")
+ params = hook._get_conn_params()
+ assert "proxy" not in params
+ assert "base_url" not in params
+
+ def test_empty_string_ignored_non_prefixed(self, monkeypatch):
+ monkeypatch.setenv(
+ "AIRFLOW_CONN_MY_CONN",
+ json.dumps({"password": "hi", "extra": {"base_url": "", "proxy":
""}}),
+ )
+ hook = SlackHook(slack_conn_id="my_conn")
+ params = hook._get_conn_params()
+ assert "proxy" not in params
+ assert "base_url" not in params
def test_default_conn_name(self):
hook = SlackHook()
assert hook.slack_conn_id == SlackHook.default_conn_name
+
+ def test_get_channel_id(self, mocked_client):
+ fake_responses = [
+ self.fake_slack_response(
+ data={
+ "channels": [
+ {"id": "C0000000000", "name":
"development-first-pr-support"},
+ {"id": "C0000000001", "name": "development"},
+ ],
+ "response_metadata": {"next_cursor": "FAKE"},
+ }
+ ),
+ self.fake_slack_response(data={"response_metadata":
{"next_cursor": "FAKE"}}),
+ self.fake_slack_response(data={"channels": [{"id": "C0000000002",
"name": "random"}]}),
+ # Below should not reach, because previous one doesn't contain
``next_cursor``
+ self.fake_slack_response(data={"channels": [{"id": "C0000000003",
"name": "troubleshooting"}]}),
+ ]
+ mocked_client.conversations_list.side_effect = fake_responses
+
+ hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
+
+ assert hook.get_channel_id("development") == "C0000000001"
+ mocked_client.conversations_list.assert_called()
+
+ mocked_client.conversations_list.reset_mock()
+ mocked_client.conversations_list.side_effect = fake_responses
+ assert hook.get_channel_id("development-first-pr-support") ==
"C0000000000"
+ # It should use cached values, so there is no new calls expected here
+ mocked_client.assert_not_called()
+
+ # Test pagination
+ mocked_client.conversations_list.side_effect = fake_responses
+ assert hook.get_channel_id("random") == "C0000000002"
+
+ # Test non-existed channels
+ mocked_client.conversations_list.side_effect = fake_responses
+ with pytest.raises(LookupError, match="Unable to find slack channel"):
+ hook.get_channel_id("troubleshooting")
+
+ def test_send_file_v2(self, mocked_client):
+ SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID).send_file_v2(
+ channel_id="C00000000", file_uploads={"file": "/foo/bar/file.txt",
"filename": "foo.txt"}
+ )
+ mocked_client.files_upload_v2.assert_called_once_with(
+ channel="C00000000",
+ file_uploads=[{"file": "/foo/bar/file.txt", "filename":
"foo.txt"}],
+ initial_comment=None,
+ )
+
+ def test_send_file_v2_multiple_files(self, mocked_client):
+ SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID).send_file_v2(
+ file_uploads=[
+ {"file": "/foo/bar/file.txt"},
+ {"content": "Some Text", "filename": "foo.txt"},
+ ],
+ initial_comment="Awesome File",
+ )
+ mocked_client.files_upload_v2.assert_called_once_with(
+ channel=None,
+ file_uploads=[
+ {"file": "/foo/bar/file.txt", "filename": "Uploaded file"},
+ {"content": "Some Text", "filename": "foo.txt"},
+ ],
+ initial_comment="Awesome File",
+ )
+
+ def test_send_file_v2_channel_name(self, mocked_client, caplog):
+ with mock.patch.object(SlackHook, "get_channel_id",
return_value="C00") as mocked_get_channel_id:
+ warning_message = "consider replacing '#random' with the
corresponding Channel ID 'C00'"
+ with pytest.warns(UserWarning, match=warning_message):
+
SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID).send_file_v2(
+ channel_id="#random", file_uploads={"file":
"/foo/bar/file.txt"}
+ )
+ mocked_get_channel_id.assert_called_once_with("random")
+ mocked_client.files_upload_v2.assert_called_once_with(
+ channel="C00",
+ file_uploads=mock.ANY,
+ initial_comment=mock.ANY,
+ )
+
+ @pytest.mark.parametrize("initial_comment", [None, "test comment"])
+ @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"])
+ def test_send_file_v1_to_v2_content(self, initial_comment, title,
filename, channel, filetype):
+ 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(
+ channels=channel,
+ content='{"foo": "bar"}',
+ filename=filename,
+ initial_comment=initial_comment,
+ title=title,
+ filetype=filetype,
+ )
+ mocked_send_file_v2.assert_called_once_with(
+ channel_id=channel,
+ file_uploads={
+ "content": '{"foo": "bar"}',
+ "filename": filename,
+ "title": title,
+ "snippet_type": filetype,
+ },
+ initial_comment=initial_comment,
+ )
+
+ @pytest.mark.parametrize("initial_comment", [None, "test comment"])
+ @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"])
+ def test_send_file_v1_to_v2_file(self, initial_comment, title, filename,
channel, filetype):
+ 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(
+ channels=channel,
+ file="/foo/bar/spam.egg",
+ filename=filename,
+ initial_comment=initial_comment,
+ title=title,
+ filetype=filetype,
+ )
+ mocked_send_file_v2.assert_called_once_with(
+ channel_id=channel,
+ file_uploads={
+ "file": "/foo/bar/spam.egg",
+ "filename": filename or "spam.egg",
+ "title": title,
+ "snippet_type": filetype,
+ },
+ initial_comment=initial_comment,
+ )
+
+ @pytest.mark.parametrize(
+ "file,content",
+ [
+ pytest.param(None, None, id="both-none"),
+ pytest.param("", "", id="both-empty"),
+ pytest.param("foo.bar", "test-content", id="both-specified"),
+ ],
+ )
+ def test_send_file_v1_to_v2_wrong_parameters(self, file, content):
+ hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID)
+ error_message = "Either `file` or `content` must be provided, not both"
+ with pytest.raises(ValueError, match=error_message):
+ hook.send_file_v1_to_v2(file=file, content=content)
+
+ @pytest.mark.parametrize(
+ "channels, expected_calls",
+ [
+ pytest.param("#foo, #bar", 2, id="comma-separated-string"),
+ pytest.param(["#random", "#development", "#airflow-upgrades"], 3,
id="list"),
+ ],
+ )
+ def test_send_file_v1_to_v2_multiple_channels(self, channels,
expected_calls):
+ 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(channels=channels, content="Fake")
+ assert mocked_send_file_v2.call_count == expected_calls
diff --git a/tests/providers/slack/operators/test_slack.py
b/tests/providers/slack/operators/test_slack.py
index 52a28ef4d8..b389f12609 100644
--- a/tests/providers/slack/operators/test_slack.py
+++ b/tests/providers/slack/operators/test_slack.py
@@ -19,7 +19,6 @@ from __future__ import annotations
import json
from unittest import mock
-from unittest.mock import MagicMock
import pytest
@@ -60,7 +59,10 @@ class TestSlackAPIOperator:
def test_hook(self, mock_slack_hook_cls, slack_op_kwargs,
hook_extra_kwargs):
mock_slack_hook = mock_slack_hook_cls.return_value
op = SlackAPIOperator(
- task_id="test-mask-token",
slack_conn_id=SLACK_API_TEST_CONNECTION_ID, **slack_op_kwargs
+ task_id="test-mask-token",
+ slack_conn_id=SLACK_API_TEST_CONNECTION_ID,
+ method="foo.Bar",
+ **slack_op_kwargs,
)
hook = op.hook
assert hook == mock_slack_hook
@@ -69,6 +71,20 @@ class TestSlackAPIOperator:
slack_conn_id=SLACK_API_TEST_CONNECTION_ID, **hook_extra_kwargs
)
+ @pytest.mark.parametrize("slack_method", [pytest.param("", id="empty"),
pytest.param(None, id="none")])
+ def test_empty_method(self, slack_method):
+ warning_message = "Define `method` parameter as empty string or None
is deprecated"
+ with pytest.warns(AirflowProviderDeprecationWarning,
match=warning_message):
+ # Should only raise a warning on task initialisation
+ op = SlackAPIOperator(
+ task_id="test-mask-token",
+ slack_conn_id=SLACK_API_TEST_CONNECTION_ID,
+ method=slack_method,
+ )
+
+ with pytest.raises(ValueError, match="Expected non empty `method`
attribute"):
+ op.execute({})
+
class TestSlackAPIPostOperator:
def setup_method(self):
@@ -158,7 +174,7 @@ class TestSlackAPIPostOperator:
slack_conn_id=SLACK_API_TEST_CONNECTION_ID,
)
- slack_api_post_operator.execute(context=MagicMock())
+ slack_api_post_operator.execute({})
expected_api_params = {
"channel": "#general",
@@ -211,47 +227,63 @@ class TestSlackAPIFileOperator:
assert slack_api_post_operator.content == self.test_content
assert not hasattr(slack_api_post_operator, "token")
- @mock.patch("airflow.providers.slack.operators.slack.SlackHook.send_file")
@pytest.mark.parametrize("initial_comment", [None, "foo-bar"])
@pytest.mark.parametrize("title", [None, "Spam Egg"])
- def test_api_call_params_with_content_args(self, mock_send_file,
initial_comment, title):
- SlackAPIFileOperator(
+ @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_api_call_params_with_content_args(self, initial_comment, title,
method_version, method_name):
+ 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,
- ).execute(context=MagicMock())
-
- mock_send_file.assert_called_once_with(
- channels="#test-channel",
- content="test-content",
- file=None,
- initial_comment=initial_comment,
- title=title,
+ method_version=method_version,
)
+ with
mock.patch(f"airflow.providers.slack.operators.slack.SlackHook.{method_name}")
as mock_send_file:
+ op.execute({})
+ mock_send_file.assert_called_once_with(
+ channels="#test-channel",
+ content="test-content",
+ file=None,
+ initial_comment=initial_comment,
+ title=title,
+ )
- @mock.patch("airflow.providers.slack.operators.slack.SlackHook.send_file")
@pytest.mark.parametrize("initial_comment", [None, "foo-bar"])
@pytest.mark.parametrize("title", [None, "Spam Egg"])
- def test_api_call_params_with_file_args(self, mock_send_file,
initial_comment, title):
- SlackAPIFileOperator(
+ @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_api_call_params_with_file_args(self, initial_comment, title,
method_version, method_name):
+ op = SlackAPIFileOperator(
task_id="slack",
slack_conn_id=SLACK_API_TEST_CONNECTION_ID,
channels="C1234567890",
filename="/dev/null",
initial_comment=initial_comment,
title=title,
- ).execute(context=MagicMock())
-
- mock_send_file.assert_called_once_with(
- channels="C1234567890",
- content=None,
- file="/dev/null",
- initial_comment=initial_comment,
- title=title,
+ method_version=method_version,
)
+ with
mock.patch(f"airflow.providers.slack.operators.slack.SlackHook.{method_name}")
as mock_send_file:
+ op.execute({})
+ mock_send_file.assert_called_once_with(
+ channels="C1234567890",
+ content=None,
+ file="/dev/null",
+ initial_comment=initial_comment,
+ title=title,
+ )
def test_channel_deprecated(self):
warning_message = (
diff --git a/tests/providers/slack/transfers/test_sql_to_slack.py
b/tests/providers/slack/transfers/test_sql_to_slack.py
index 9d60f28b42..fd3c06189d 100644
--- a/tests/providers/slack/transfers/test_sql_to_slack.py
+++ b/tests/providers/slack/transfers/test_sql_to_slack.py
@@ -80,6 +80,13 @@ class TestSqlToSlackApiFileOperator:
),
],
)
+ @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,
@@ -92,10 +99,12 @@ class TestSqlToSlackApiFileOperator:
title,
slack_op_kwargs: dict,
hook_extra_kwargs: dict,
+ method_version,
+ method_name: str,
):
# Mock Hook
mock_send_file = mock.MagicMock()
- mock_slack_hook_cls.return_value.send_file = mock_send_file
+ setattr(mock_slack_hook_cls.return_value, method_name, mock_send_file)
# Mock returns pandas.DataFrame and expected method
mock_df = mock.MagicMock()
@@ -110,10 +119,13 @@ class TestSqlToSlackApiFileOperator:
"slack_channels": channels,
"slack_initial_comment": initial_comment,
"slack_title": title,
+ "slack_method_version": method_version,
"df_kwargs": df_kwargs,
**slack_op_kwargs,
}
op = SqlToSlackApiFileOperator(task_id="test_send_file", **op_kwargs)
+
+ mock.patch("airflow.providers.slack.transfers.sql_to_slack.SlackHook")
op.execute(mock.MagicMock())
mock_slack_hook_cls.assert_called_once_with(
diff --git a/tests/system/providers/slack/example_slack.py
b/tests/system/providers/slack/example_slack.py
index bbb47374b3..78bd649798 100644
--- a/tests/system/providers/slack/example_slack.py
+++ b/tests/system/providers/slack/example_slack.py
@@ -83,6 +83,7 @@ with DAG(
task_id="slack_file_upload_2",
channels=SLACK_CHANNEL,
content="file content in txt",
+ method_version="v2",
)
# [END slack_api_file_operator_content_howto_guide]
