From 271265b80ba07e4546774cd213043ffb3c0c65e9 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Fri, 13 Aug 2021 18:05:59 +0900 Subject: [PATCH] Improve #1084 by removing globally mutable lists --- slack_sdk/audit_logs/v1/async_client.py | 2 +- slack_sdk/audit_logs/v1/client.py | 2 +- slack_sdk/http_retry/__init__.py | 18 +++++++------ .../http_retry/builtin_async_handlers.py | 3 ++- slack_sdk/scim/v1/async_client.py | 2 +- slack_sdk/scim/v1/client.py | 2 +- slack_sdk/web/async_base_client.py | 2 +- slack_sdk/web/base_client.py | 2 +- slack_sdk/webhook/async_client.py | 2 +- slack_sdk/webhook/client.py | 2 +- tests/slack_sdk/http_retry/test_builtins.py | 25 +++++++++++++------ tests/slack_sdk_async/http_retry/__init__.py | 0 .../http_retry/test_builtins.py | 13 ++++++++++ 13 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 tests/slack_sdk_async/http_retry/__init__.py create mode 100644 tests/slack_sdk_async/http_retry/test_builtins.py diff --git a/slack_sdk/audit_logs/v1/async_client.py b/slack_sdk/audit_logs/v1/async_client.py index 13c543bc5..49dc937e7 100644 --- a/slack_sdk/audit_logs/v1/async_client.py +++ b/slack_sdk/audit_logs/v1/async_client.py @@ -55,7 +55,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[AsyncRetryHandler] = async_default_handlers, + retry_handlers: List[AsyncRetryHandler] = async_default_handlers(), ): """API client for Audit Logs API See https://api.slack.com/admins/audit-logs for more details diff --git a/slack_sdk/audit_logs/v1/client.py b/slack_sdk/audit_logs/v1/client.py index fcd035dc5..ad3479de6 100644 --- a/slack_sdk/audit_logs/v1/client.py +++ b/slack_sdk/audit_logs/v1/client.py @@ -50,7 +50,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[RetryHandler] = default_retry_handlers, + retry_handlers: List[RetryHandler] = default_retry_handlers(), ): """API client for Audit Logs API See https://api.slack.com/admins/audit-logs for more details diff --git a/slack_sdk/http_retry/__init__.py b/slack_sdk/http_retry/__init__.py index 83c6315cf..3f0367ab3 100644 --- a/slack_sdk/http_retry/__init__.py +++ b/slack_sdk/http_retry/__init__.py @@ -1,3 +1,5 @@ +from typing import List + from .handler import RetryHandler # noqa from .builtin_handlers import ( ConnectionErrorRetryHandler, # noqa @@ -16,11 +18,13 @@ connect_error_retry_handler = ConnectionErrorRetryHandler() # noqa rate_limit_error_retry_handler = RateLimitErrorRetryHandler() # noqa -default_retry_handlers = [ # noqa - connect_error_retry_handler, # noqa -] # noqa -all_builtin_retry_handlers = [ # noqa - connect_error_retry_handler, # noqa - rate_limit_error_retry_handler, # noqa -] # noqa +def default_retry_handlers() -> List[RetryHandler]: + return [connect_error_retry_handler] + + +def all_builtin_retry_handlers() -> List[RetryHandler]: + return [ + connect_error_retry_handler, + rate_limit_error_retry_handler, + ] diff --git a/slack_sdk/http_retry/builtin_async_handlers.py b/slack_sdk/http_retry/builtin_async_handlers.py index afc42cfd0..9d381de47 100644 --- a/slack_sdk/http_retry/builtin_async_handlers.py +++ b/slack_sdk/http_retry/builtin_async_handlers.py @@ -87,4 +87,5 @@ async def prepare_for_next_attempt_async( await asyncio.sleep(duration) -async_default_handlers = [AsyncConnectionErrorRetryHandler()] +def async_default_handlers() -> List[AsyncRetryHandler]: + return [AsyncConnectionErrorRetryHandler()] diff --git a/slack_sdk/scim/v1/async_client.py b/slack_sdk/scim/v1/async_client.py index aaa8b6cea..ce20cef56 100644 --- a/slack_sdk/scim/v1/async_client.py +++ b/slack_sdk/scim/v1/async_client.py @@ -70,7 +70,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[AsyncRetryHandler] = async_default_handlers, + retry_handlers: List[AsyncRetryHandler] = async_default_handlers(), ): """API client for SCIM API See https://api.slack.com/scim for more details diff --git a/slack_sdk/scim/v1/client.py b/slack_sdk/scim/v1/client.py index aabbf7b74..e10513626 100644 --- a/slack_sdk/scim/v1/client.py +++ b/slack_sdk/scim/v1/client.py @@ -72,7 +72,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[RetryHandler] = default_retry_handlers, + retry_handlers: List[RetryHandler] = default_retry_handlers(), ): """API client for SCIM API See https://api.slack.com/scim for more details diff --git a/slack_sdk/web/async_base_client.py b/slack_sdk/web/async_base_client.py index 0344c364f..bf0d5fd59 100644 --- a/slack_sdk/web/async_base_client.py +++ b/slack_sdk/web/async_base_client.py @@ -41,7 +41,7 @@ def __init__( # for Org-Wide App installation team_id: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[RetryHandler] = async_default_handlers, + retry_handlers: List[RetryHandler] = async_default_handlers(), ): self.token = None if token is None else token.strip() self.base_url = base_url diff --git a/slack_sdk/web/base_client.py b/slack_sdk/web/base_client.py index e5ca3910a..75b16cc0e 100644 --- a/slack_sdk/web/base_client.py +++ b/slack_sdk/web/base_client.py @@ -54,7 +54,7 @@ def __init__( # for Org-Wide App installation team_id: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[RetryHandler] = default_retry_handlers, + retry_handlers: List[RetryHandler] = default_retry_handlers(), ): self.token = None if token is None else token.strip() self.base_url = base_url diff --git a/slack_sdk/webhook/async_client.py b/slack_sdk/webhook/async_client.py index 2bb9628c6..24b36d361 100644 --- a/slack_sdk/webhook/async_client.py +++ b/slack_sdk/webhook/async_client.py @@ -49,7 +49,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[AsyncRetryHandler] = async_default_handlers, + retry_handlers: List[AsyncRetryHandler] = async_default_handlers(), ): """API client for Incoming Webhooks and `response_url` diff --git a/slack_sdk/webhook/client.py b/slack_sdk/webhook/client.py index 490cb7e79..0d7157f5c 100644 --- a/slack_sdk/webhook/client.py +++ b/slack_sdk/webhook/client.py @@ -44,7 +44,7 @@ def __init__( user_agent_prefix: Optional[str] = None, user_agent_suffix: Optional[str] = None, logger: Optional[logging.Logger] = None, - retry_handlers: List[RetryHandler] = default_retry_handlers, + retry_handlers: List[RetryHandler] = default_retry_handlers(), ): """API client for Incoming Webhooks and `response_url` diff --git a/tests/slack_sdk/http_retry/test_builtins.py b/tests/slack_sdk/http_retry/test_builtins.py index 8c157aad9..49394bb30 100644 --- a/tests/slack_sdk/http_retry/test_builtins.py +++ b/tests/slack_sdk/http_retry/test_builtins.py @@ -1,18 +1,27 @@ import unittest -from slack_sdk.http_retry import FixedValueRetryIntervalCalculator -from tests.slack_sdk.audit_logs.mock_web_api_server import ( - cleanup_mock_web_api_server, - setup_mock_web_api_server, +from slack_sdk.http_retry import ( + FixedValueRetryIntervalCalculator, + default_retry_handlers, + all_builtin_retry_handlers, ) class TestBuiltins(unittest.TestCase): - def setUp(self): - setup_mock_web_api_server(self) + def test_default_ones(self): + list = default_retry_handlers() + self.assertEqual(1, len(list)) + list.clear() + self.assertEqual(0, len(list)) + list = default_retry_handlers() + self.assertEqual(1, len(list)) - def tearDown(self): - cleanup_mock_web_api_server(self) + list = all_builtin_retry_handlers() + self.assertEqual(2, len(list)) + list.clear() + self.assertEqual(0, len(list)) + list = all_builtin_retry_handlers() + self.assertEqual(2, len(list)) def test_fixed_value_retry_interval_calculator(self): for fixed_value in [0.1, 0.2]: diff --git a/tests/slack_sdk_async/http_retry/__init__.py b/tests/slack_sdk_async/http_retry/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/slack_sdk_async/http_retry/test_builtins.py b/tests/slack_sdk_async/http_retry/test_builtins.py new file mode 100644 index 000000000..45c086e61 --- /dev/null +++ b/tests/slack_sdk_async/http_retry/test_builtins.py @@ -0,0 +1,13 @@ +import unittest + +from slack_sdk.http_retry.builtin_async_handlers import async_default_handlers + + +class TestBuiltins(unittest.TestCase): + def test_default_ones(self): + list = async_default_handlers() + self.assertEqual(1, len(list)) + list.clear() + self.assertEqual(0, len(list)) + list = async_default_handlers() + self.assertEqual(1, len(list))