From cc25d71391cf56525d3f9b3c8ed1613b5c8ba94d Mon Sep 17 00:00:00 2001 From: Andrey Anshin Date: Wed, 1 May 2024 13:03:52 +0400 Subject: [PATCH] Use `upload_files_v2` Slack SDK method by default in Slack Operators --- airflow/providers/slack/CHANGELOG.rst | 17 +++++++ airflow/providers/slack/hooks/slack.py | 11 ++++- .../providers/slack/transfers/sql_to_slack.py | 2 +- .../operators/slack_api.rst | 10 ++-- .../operators/sql_to_slack.rst | 11 +++-- tests/providers/slack/hooks/test_slack.py | 48 +++++++++++-------- tests/system/providers/slack/example_slack.py | 1 - 7 files changed, 70 insertions(+), 30 deletions(-) diff --git a/airflow/providers/slack/CHANGELOG.rst b/airflow/providers/slack/CHANGELOG.rst index 97ce81b5240fff..bed7c0cbe15d0b 100644 --- a/airflow/providers/slack/CHANGELOG.rst +++ b/airflow/providers/slack/CHANGELOG.rst @@ -27,6 +27,23 @@ Changelog --------- +.. note:: + Due to future discontinue of `files.upload `__ + Slack API method the default value of ``SlackAPIFileOperator.method_version`` and ``SqlToSlackApiFileOperator.slack_method_version`` + changed from ``v1`` to ``v2`` + + If you previously use ``v1`` you should check that your application has appropriate scopes: + + * **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** + + If you use ``SlackHook.send_file`` please consider switch to ``SlackHook.send_file_v2`` + or ``SlackHook.send_file_v1_to_v2`` methods. + 8.6.2 ..... diff --git a/airflow/providers/slack/hooks/slack.py b/airflow/providers/slack/hooks/slack.py index f046a9c5fcd036..e5d4e1bb876c19 100644 --- a/airflow/providers/slack/hooks/slack.py +++ b/airflow/providers/slack/hooks/slack.py @@ -23,11 +23,12 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Sequence, TypedDict +from deprecated import deprecated from slack_sdk import WebClient from slack_sdk.errors import SlackApiError from typing_extensions import NotRequired -from airflow.exceptions import AirflowNotFoundException +from airflow.exceptions import AirflowNotFoundException, AirflowProviderDeprecationWarning from airflow.hooks.base import BaseHook from airflow.providers.slack.utils import ConnectionExtraConfig from airflow.utils.helpers import exactly_one @@ -184,6 +185,14 @@ def call(self, api_method: str, **kwargs) -> SlackResponse: """ return self.client.api_call(api_method, **kwargs) + @deprecated( + reason=( + "This method utilise `files.upload` Slack API method which is being sunset on March 11, 2025. " + "Beginning May 8, 2024, newly-created apps will be unable to 'files.upload' Slack API. " + "Please use `send_file_v2` or `send_file_v1_to_v2` instead." + ), + category=AirflowProviderDeprecationWarning, + ) def send_file( self, *, diff --git a/airflow/providers/slack/transfers/sql_to_slack.py b/airflow/providers/slack/transfers/sql_to_slack.py index cd19fa9a745d07..6b3ebeec86bf35 100644 --- a/airflow/providers/slack/transfers/sql_to_slack.py +++ b/airflow/providers/slack/transfers/sql_to_slack.py @@ -90,7 +90,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"] = "v1", + slack_method_version: Literal["v1", "v2"] = "v2", df_kwargs: dict | None = None, action_on_empty_df: Literal["send", "skip", "error"] = "send", **kwargs, diff --git a/docs/apache-airflow-providers-slack/operators/slack_api.rst b/docs/apache-airflow-providers-slack/operators/slack_api.rst index 6b2debb405c398..6fafa46f79418f 100644 --- a/docs/apache-airflow-providers-slack/operators/slack_api.rst +++ b/docs/apache-airflow-providers-slack/operators/slack_api.rst @@ -60,9 +60,12 @@ 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: + by default it use Slack SDK method ``upload_files_v2`` it is possible to use legacy ``upload_files`` + method by set ``v1`` to ``method_version`` however this not recommended because it + might impact a performance, cause random API errors and is being sunset on March 11, 2025 in addition + beginning May 8, 2024, newly-created apps will be unable to use this API method. + + If you previously use ``v1`` you should check that your application has appropriate scopes: * **files:write** - for write files. * **files:read** - for read files (not required if you use Slack SDK >= 3.23.0). @@ -74,6 +77,7 @@ Using the Operator .. seealso:: - `Slack SDK 3.19.0 Release Notes `_ - `conversations.list API `_ + - `files.upload retires in March 2025 `_ You could send file attachment by specifying file path 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 2bf053f5a8b352..3182fd7e0b785b 100644 --- a/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst +++ b/docs/apache-airflow-providers-slack/operators/sql_to_slack.rst @@ -28,9 +28,12 @@ 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: + by default it use Slack SDK method ``upload_files_v2`` it is possible to use legacy ``upload_files`` + method by set ``v1`` to ``slack_method_version`` however this not recommended because it + might impact a performance, cause random API errors and is being sunset on March 11, 2025 in addition + beginning May 8, 2024, newly-created apps will be unable to use this API method. + + If you previously use ``v1`` you should check that your application has appropriate scopes: * **files:write** - for write files. * **files:read** - for read files (not required if you use Slack SDK >= 3.23.0). @@ -42,7 +45,7 @@ Using the Operator .. seealso:: - `Slack SDK 3.19.0 Release Notes `_ - `conversations.list API `_ - + - `files.upload retires in March 2025 `_ This operator will execute a custom query in the provided SQL connection and publish a file to Slack channel(s). diff --git a/tests/providers/slack/hooks/test_slack.py b/tests/providers/slack/hooks/test_slack.py index 023c6a9ebd143b..35dfa5200b7faa 100644 --- a/tests/providers/slack/hooks/test_slack.py +++ b/tests/providers/slack/hooks/test_slack.py @@ -26,7 +26,7 @@ from slack_sdk.http_retry.builtin_handlers import ConnectionErrorRetryHandler, RateLimitErrorRetryHandler from slack_sdk.web.slack_response import SlackResponse -from airflow.exceptions import AirflowNotFoundException +from airflow.exceptions import AirflowNotFoundException, AirflowProviderDeprecationWarning from airflow.models.connection import Connection from airflow.providers.slack.hooks.slack import SlackHook @@ -307,9 +307,11 @@ def test_hook_connection_failed(self, mocked_client, response_data): ) def test_send_file_wrong_parameters(self, file, content): hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) + warning_message = "use `send_file_v2` or `send_file_v1_to_v2" 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) + with pytest.warns(AirflowProviderDeprecationWarning, match=warning_message): + with pytest.raises(ValueError, match=error_message): + hook.send_file(file=file, content=content) @pytest.mark.parametrize("initial_comment", [None, "test comment"]) @pytest.mark.parametrize("title", [None, "test title"]) @@ -324,14 +326,16 @@ def test_send_file_path( file.write_text('{"foo": "bar"}') hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) - hook.send_file( - channels=channels, - file=file, - filename="filename.mock", - initial_comment=initial_comment, - title=title, - filetype=filetype, - ) + warning_message = "use `send_file_v2` or `send_file_v1_to_v2" + with pytest.warns(AirflowProviderDeprecationWarning, match=warning_message): + hook.send_file( + channels=channels, + file=file, + filename="filename.mock", + initial_comment=initial_comment, + title=title, + filetype=filetype, + ) mocked_client.files_upload.assert_called_once_with( channels=channels, @@ -355,7 +359,9 @@ def test_send_file_path_set_filename(self, mocked_client, tmp_path_factory, file file.write_text('{"foo": "bar"}') hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) - hook.send_file(file=file) + warning_message = "use `send_file_v2` or `send_file_v1_to_v2" + with pytest.warns(AirflowProviderDeprecationWarning, match=warning_message): + hook.send_file(file=file) assert mocked_client.files_upload.call_count == 1 call_args = mocked_client.files_upload.call_args.kwargs @@ -370,14 +376,16 @@ def test_send_file_path_set_filename(self, mocked_client, tmp_path_factory, file def test_send_file_content(self, mocked_client, initial_comment, title, filetype, channels, filename): """Test send file by providing content.""" hook = SlackHook(slack_conn_id=SLACK_API_DEFAULT_CONN_ID) - hook.send_file( - channels=channels, - content='{"foo": "bar"}', - filename=filename, - initial_comment=initial_comment, - title=title, - filetype=filetype, - ) + warning_message = "use `send_file_v2` or `send_file_v1_to_v2" + with pytest.warns(AirflowProviderDeprecationWarning, match=warning_message): + hook.send_file( + channels=channels, + content='{"foo": "bar"}', + filename=filename, + initial_comment=initial_comment, + title=title, + filetype=filetype, + ) mocked_client.files_upload.assert_called_once_with( channels=channels, content='{"foo": "bar"}', diff --git a/tests/system/providers/slack/example_slack.py b/tests/system/providers/slack/example_slack.py index 78bd649798ccb4..bbb47374b37d4a 100644 --- a/tests/system/providers/slack/example_slack.py +++ b/tests/system/providers/slack/example_slack.py @@ -83,7 +83,6 @@ 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]