You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2022/09/19 12:07:18 UTC
[airflow] branch main updated: Remove unsafe imports in Slack API Connection (#26459)
This is an automated email from the ASF dual-hosted git repository.
potiuk 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 7d5e8cce6c Remove unsafe imports in Slack API Connection (#26459)
7d5e8cce6c is described below
commit 7d5e8cce6c3e27bc7b9bf28823e9adc6cdb458e2
Author: Andrey Anshin <An...@taragol.is>
AuthorDate: Mon Sep 19 16:07:08 2022 +0400
Remove unsafe imports in Slack API Connection (#26459)
---
airflow/providers/slack/hooks/slack.py | 15 +-------------
airflow/providers/slack/utils/__init__.py | 21 -------------------
.../connections/slack.rst | 2 --
tests/providers/slack/hooks/test_slack.py | 24 ----------------------
tests/providers/slack/utils/test_utils.py | 13 ------------
5 files changed, 1 insertion(+), 74 deletions(-)
diff --git a/airflow/providers/slack/hooks/slack.py b/airflow/providers/slack/hooks/slack.py
index 43c41bf523..5d280a6c5b 100644
--- a/airflow/providers/slack/hooks/slack.py
+++ b/airflow/providers/slack/hooks/slack.py
@@ -135,7 +135,7 @@ class SlackHook(BaseHook):
def _get_conn_params(self) -> dict[str, Any]:
"""Fetch connection params as a dict and merge it with hook parameters."""
conn = self.get_connection(self.slack_conn_id) if self.slack_conn_id else None
- conn_params: dict[str, Any] = {}
+ conn_params: dict[str, Any] = {"retry_handlers": self.retry_handlers}
if self._token:
conn_params["token"] = self._token
@@ -158,9 +158,6 @@ class SlackHook(BaseHook):
"timeout": self.timeout or extra_config.getint("timeout", default=None),
"base_url": self.base_url or extra_config.get("base_url", default=None),
"proxy": self.proxy or extra_config.get("proxy", default=None),
- "retry_handlers": (
- self.retry_handlers or extra_config.getimports("retry_handlers", default=None)
- ),
}
)
@@ -314,12 +311,6 @@ class SlackHook(BaseHook):
widget=BS3TextFieldWidget(),
description="Optional. Proxy to make the Slack API call.",
),
- prefixed_extra_field("retry_handlers", cls.conn_type): StringField(
- lazy_gettext('Retry Handlers'),
- widget=BS3TextFieldWidget(),
- description="Optional. Comma separated list of import paths to zero-argument callable "
- "which returns retry handler for Slack WebClient.",
- ),
}
@classmethod
@@ -335,9 +326,5 @@ class SlackHook(BaseHook):
prefixed_extra_field("timeout", cls.conn_type): "30",
prefixed_extra_field("base_url", cls.conn_type): "https://www.slack.com/api/",
prefixed_extra_field("proxy", cls.conn_type): "http://localhost:9000",
- prefixed_extra_field("retry_handlers", cls.conn_type): (
- "slack_sdk.http_retry.builtin_handlers.ConnectionErrorRetryHandler,"
- "slack_sdk.http_retry.builtin_handlers.RateLimitErrorRetryHandler"
- ),
},
}
diff --git a/airflow/providers/slack/utils/__init__.py b/airflow/providers/slack/utils/__init__.py
index a1283f8983..bfb9478ab3 100644
--- a/airflow/providers/slack/utils/__init__.py
+++ b/airflow/providers/slack/utils/__init__.py
@@ -18,8 +18,6 @@ from __future__ import annotations
from typing import Any
-from airflow.utils.module_loading import import_string
-
try:
from airflow.utils.types import NOTSET
except ImportError: # TODO: Remove when the provider has an Airflow 2.3+ requirement.
@@ -78,22 +76,3 @@ class ConnectionExtraConfig:
if value != default:
value = int(value)
return value
-
- def getimports(self, field, default: Any = NOTSET) -> Any:
- """Get specified field from Connection Extra and imports the full qualified name separated by comma.
-
- .. note::
- This method intends to use with zero-argument callable objects.
-
- :param field: Connection extra field name.
- :param default: If specified then use as default value if field not present in Connection Extra.
- """
- value = self.get(field=field, default=default)
- if value != default:
- if not isinstance(value, str):
- raise TypeError(
- f"Connection ({self.conn_id!r}) Extra {field!r} expected string "
- f"when return value not equal `default={default}`, got {type(value).__name__}."
- )
- value = [import_string(part.strip())() for part in value.split(",")]
- return value
diff --git a/docs/apache-airflow-providers-slack/connections/slack.rst b/docs/apache-airflow-providers-slack/connections/slack.rst
index ffcb128660..c07ea2191f 100644
--- a/docs/apache-airflow-providers-slack/connections/slack.rst
+++ b/docs/apache-airflow-providers-slack/connections/slack.rst
@@ -51,8 +51,6 @@ Extra (optional)
* ``timeout``: The maximum number of seconds the client will wait to connect and receive a response from Slack API.
* ``base_url``: A string representing the Slack API base URL.
* ``proxy``: Proxy to make the Slack Incoming Webhook call.
- * ``retry_handlers``: Comma separated list of import paths to zero-argument callable which returns retry handler
- for Slack WebClient.
If you are configuring the connection via a URI, ensure that all components of the URI are URL-encoded.
diff --git a/tests/providers/slack/hooks/test_slack.py b/tests/providers/slack/hooks/test_slack.py
index a668b1510a..9dde301dce 100644
--- a/tests/providers/slack/hooks/test_slack.py
+++ b/tests/providers/slack/hooks/test_slack.py
@@ -44,14 +44,6 @@ VALID_CONN_IDS = [
]
-def conn_error_retry_handler():
- return TEST_CONN_ERROR_RETRY_HANDLER
-
-
-def rate_limit_retry_handler():
- return TEST_RATE_LIMIT_RETRY_HANDLER
-
-
@pytest.fixture(scope="module", autouse=True)
def slack_api_connections():
"""Create tests connections."""
@@ -170,16 +162,11 @@ class TestSlackHook:
"timeout": "9000",
"base_url": "http://conn-base-url:4321",
"proxy": "https://conn-proxy:4321",
- "retry_handlers": (
- "tests.providers.slack.hooks.test_slack.rate_limit_retry_handler,"
- "tests.providers.slack.hooks.test_slack.conn_error_retry_handler"
- ),
},
{
"timeout": 9000,
"base_url": "http://conn-base-url:4321",
"proxy": "https://conn-proxy:4321",
- "retry_handlers": [TEST_RATE_LIMIT_RETRY_HANDLER, TEST_CONN_ERROR_RETRY_HANDLER],
},
),
( # Test Case: Connection from the UI
@@ -188,16 +175,11 @@ class TestSlackHook:
"extra__slack__timeout": 9000,
"extra__slack__base_url": "http://conn-base-url:4321",
"extra__slack__proxy": "https://conn-proxy:4321",
- "extra__slack__retry_handlers": (
- "tests.providers.slack.hooks.test_slack.rate_limit_retry_handler,"
- "tests.providers.slack.hooks.test_slack.conn_error_retry_handler"
- ),
},
{
"timeout": 9000,
"base_url": "http://conn-base-url:4321",
"proxy": "https://conn-proxy:4321",
- "retry_handlers": [TEST_RATE_LIMIT_RETRY_HANDLER, TEST_CONN_ERROR_RETRY_HANDLER],
},
),
( # Test Case: Merge configs - hook args overwrite conn config
@@ -209,10 +191,6 @@ class TestSlackHook:
{
"timeout": 9000,
"proxy": "https://conn-proxy:4321",
- "retry_handlers": (
- "tests.providers.slack.hooks.test_slack.rate_limit_retry_handler,"
- "tests.providers.slack.hooks.test_slack.conn_error_retry_handler"
- ),
},
{
"timeout": 1,
@@ -227,12 +205,10 @@ class TestSlackHook:
{
"timeout": 9000,
"proxy": "https://conn-proxy:4334",
- "retry_handlers": ("tests.providers.slack.hooks.test_slack.conn_error_retry_handler"),
},
{
"timeout": 1,
"proxy": "https://conn-proxy:4334",
- "retry_handlers": [TEST_CONN_ERROR_RETRY_HANDLER],
},
),
( # Test Case: empty configs
diff --git a/tests/providers/slack/utils/test_utils.py b/tests/providers/slack/utils/test_utils.py
index 081737bc32..eb45ac2f6e 100644
--- a/tests/providers/slack/utils/test_utils.py
+++ b/tests/providers/slack/utils/test_utils.py
@@ -69,16 +69,3 @@ class TestConnectionExtra:
)
assert extra_config.getint("int_arg_1") == 42
assert extra_config.getint("int_arg_2") == 9000
-
- def test_get_parse_imports(self):
- extra_config = ConnectionExtraConfig(
- conn_type="slack",
- extra={
- "imports_arg_1": "builtins.str",
- "imports_arg_2": "builtins.str,builtins.dict",
- "imports_arg_3": " builtins.str , builtins.dict ",
- },
- )
- assert extra_config.getimports("imports_arg_1") == ['']
- assert extra_config.getimports("imports_arg_2") == ['', {}]
- assert extra_config.getimports("imports_arg_3") == ['', {}]