From 24ff0daa8b4abda0a3057f289568140ba70c3dd3 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 25 Aug 2022 15:14:51 +0900 Subject: [PATCH 1/2] Fix #709 Enable developers to inject non-bolt arguments to listener function args --- slack_bolt/app/app.py | 9 +++++++-- slack_bolt/app/async_app.py | 9 +++++++-- slack_bolt/authorization/async_authorize.py | 7 +++++-- slack_bolt/authorization/authorize.py | 7 +++++-- slack_bolt/kwargs_injection/async_utils.py | 9 ++++++--- slack_bolt/kwargs_injection/utils.py | 9 ++++++--- slack_bolt/warning/__init__.py | 11 +++++++++++ 7 files changed, 47 insertions(+), 14 deletions(-) create mode 100644 slack_bolt/warning/__init__.py diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index 8a35103da..8d94939ff 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -3,6 +3,7 @@ import logging import os import time +import warnings from concurrent.futures import Executor from concurrent.futures.thread import ThreadPoolExecutor from http.server import SimpleHTTPRequestHandler, HTTPServer @@ -79,6 +80,7 @@ get_boot_message, get_name_for_callable, ) +from slack_bolt.warning import BoltCodeWarning from slack_bolt.workflows.step import WorkflowStep, WorkflowStepMiddleware from slack_bolt.workflows.step.step import WorkflowStepBuilder @@ -491,7 +493,9 @@ def middleware_next(): response=resp, ) return resp - self._framework_logger.warning(warning_unhandled_by_global_middleware(middleware.name, req)) + # In most cases, this could be a coding error, but it can be intentional for some reason + # So we use BoltCodeWarning for it + warnings.warn(warning_unhandled_by_global_middleware(middleware.name, req), BoltCodeWarning) return resp return resp @@ -552,7 +556,8 @@ def middleware_next(): return resp def _handle_unmatched_requests(self, req: BoltRequest, resp: BoltResponse) -> BoltResponse: - self._framework_logger.warning(warning_unhandled_request(req)) + # This warning is helpful in coding phase, but for production operations + warnings.warn(warning_unhandled_request(req), BoltCodeWarning) return resp # ------------------------- diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index c2db2e08e..cfa705b26 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -2,6 +2,7 @@ import logging import os import time +import warnings from typing import Optional, List, Union, Callable, Pattern, Dict, Awaitable, Sequence from aiohttp import web @@ -24,6 +25,7 @@ ) from slack_bolt.oauth.async_internals import select_consistent_installation_store from slack_bolt.util.utils import get_name_for_callable +from slack_bolt.warning import BoltCodeWarning from slack_bolt.workflows.step.async_step import ( AsyncWorkflowStep, AsyncWorkflowStepBuilder, @@ -517,7 +519,9 @@ async def async_middleware_next(): response=resp, ) return resp - self._framework_logger.warning(warning_unhandled_by_global_middleware(middleware.name, req)) + # In most cases, this could be a coding error, but it can be intentional for some reason + # So we use BoltCodeWarning for it + warnings.warn(warning_unhandled_by_global_middleware(middleware.name, req), BoltCodeWarning) return resp return resp @@ -582,7 +586,8 @@ async def async_middleware_next(): return resp def _handle_unmatched_requests(self, req: AsyncBoltRequest, resp: BoltResponse) -> BoltResponse: - self._framework_logger.warning(warning_unhandled_request(req)) + # This warning is helpful in coding phase, but for production operations + warnings.warn(warning_unhandled_request(req), BoltCodeWarning) return resp # ------------------------- diff --git a/slack_bolt/authorization/async_authorize.py b/slack_bolt/authorization/async_authorize.py index 315558892..25756a6c4 100644 --- a/slack_bolt/authorization/async_authorize.py +++ b/slack_bolt/authorization/async_authorize.py @@ -1,3 +1,4 @@ +import warnings from logging import Logger from typing import Optional, Callable, Awaitable, Dict, Any @@ -14,6 +15,7 @@ from slack_bolt.context.async_context import AsyncBoltContext from slack_bolt.error import BoltError from slack_bolt.util.utils import get_arg_names_of_callable +from slack_bolt.warning import BoltCodeWarning class AsyncAuthorize: @@ -77,8 +79,9 @@ async def __call__( found_arg_names = kwargs.keys() for name in self.arg_names: if name not in found_arg_names: - self.logger.warning(f"{name} is not a valid argument") - kwargs[name] = None + warnings.warn( + f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning + ) auth_result: Optional[AuthorizeResult] = await self.func(**kwargs) if auth_result is None: diff --git a/slack_bolt/authorization/authorize.py b/slack_bolt/authorization/authorize.py index a382f7097..b6f4294aa 100644 --- a/slack_bolt/authorization/authorize.py +++ b/slack_bolt/authorization/authorize.py @@ -1,3 +1,4 @@ +import warnings from logging import Logger from typing import Optional, Callable, Dict, Any @@ -13,6 +14,7 @@ from slack_bolt.context.context import BoltContext from slack_bolt.error import BoltError from slack_bolt.util.utils import get_arg_names_of_callable +from slack_bolt.warning import BoltCodeWarning class Authorize: @@ -81,8 +83,9 @@ def __call__( found_arg_names = kwargs.keys() for name in self.arg_names: if name not in found_arg_names: - self.logger.warning(f"{name} is not a valid argument") - kwargs[name] = None + warnings.warn( + f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning + ) auth_result = self.func(**kwargs) if auth_result is None: diff --git a/slack_bolt/kwargs_injection/async_utils.py b/slack_bolt/kwargs_injection/async_utils.py index eceb5af8e..02da4b651 100644 --- a/slack_bolt/kwargs_injection/async_utils.py +++ b/slack_bolt/kwargs_injection/async_utils.py @@ -1,6 +1,7 @@ # pytype: skip-file import inspect import logging +import warnings from typing import Callable, Dict, Optional, Any, Sequence from slack_bolt.request.async_request import AsyncBoltRequest @@ -17,6 +18,7 @@ to_step, ) from ..logger.messages import warning_skip_uncommon_arg_name +from ..warning import BoltCodeWarning def build_async_required_kwargs( @@ -85,7 +87,8 @@ def build_async_required_kwargs( required_arg_names.pop(0) elif first_arg_name not in all_available_args.keys(): if this_func is None: - logger.warning(warning_skip_uncommon_arg_name(first_arg_name)) + # Actually, it's rare to see this warning + warnings.warn(warning_skip_uncommon_arg_name(first_arg_name), BoltCodeWarning) required_arg_names.pop(0) elif inspect.ismethod(this_func): # We are sure that we should skip manipulating this arg @@ -98,9 +101,9 @@ def build_async_required_kwargs( if isinstance(request, AsyncBoltRequest): kwargs[name] = AsyncArgs(**all_available_args) else: + # We don't use BolCodeWarning here because something unexpected (e.g., bolt-python bug) may be the cause logger.warning(f"Unknown Request object type detected ({type(request)})") if name not in found_arg_names: - logger.warning(f"{name} is not a valid argument") - kwargs[name] = None + warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning) return kwargs diff --git a/slack_bolt/kwargs_injection/utils.py b/slack_bolt/kwargs_injection/utils.py index 81c4276c5..99d97832d 100644 --- a/slack_bolt/kwargs_injection/utils.py +++ b/slack_bolt/kwargs_injection/utils.py @@ -1,6 +1,7 @@ # pytype: skip-file import inspect import logging +import warnings from typing import Callable, Dict, Optional, Any, Sequence from slack_bolt.request import BoltRequest @@ -17,6 +18,7 @@ to_step, ) from ..logger.messages import warning_skip_uncommon_arg_name +from ..warning import BoltCodeWarning def build_required_kwargs( @@ -85,7 +87,8 @@ def build_required_kwargs( required_arg_names.pop(0) elif first_arg_name not in all_available_args.keys(): if this_func is None: - logger.warning(warning_skip_uncommon_arg_name(first_arg_name)) + # Actually, it's rare to see this warning + warnings.warn(warning_skip_uncommon_arg_name(first_arg_name), BoltCodeWarning) required_arg_names.pop(0) elif inspect.ismethod(this_func): # We are sure that we should skip manipulating this arg @@ -98,9 +101,9 @@ def build_required_kwargs( if isinstance(request, BoltRequest): kwargs[name] = Args(**all_available_args) else: + # We don't use BolCodeWarning here because something unexpected (e.g., bolt-python bug) may be the cause logger.warning(f"Unknown Request object type detected ({type(request)})") if name not in found_arg_names: - logger.warning(f"{name} is not a valid argument") - kwargs[name] = None + warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning) return kwargs diff --git a/slack_bolt/warning/__init__.py b/slack_bolt/warning/__init__.py new file mode 100644 index 000000000..822c4b02a --- /dev/null +++ b/slack_bolt/warning/__init__.py @@ -0,0 +1,11 @@ +"""Warnings by Bolt for Python + +Developers can ignore the following warning categories as necessary. +see also: https://docs.python.org/3/library/warnings.html#default-warning-filter +""" + + +class BoltCodeWarning(UserWarning): + """Warning to help developers notice their coding errors. + This warning should not be used for informing configuration errors in the App/AsyncApp constructor. + """ From 2c09cc64ed51296d4400843174cf5da7e191987d Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 25 Aug 2022 16:29:17 +0900 Subject: [PATCH 2/2] Delete filtering --- slack_bolt/authorization/async_authorize.py | 4 +--- slack_bolt/authorization/authorize.py | 4 +--- slack_bolt/kwargs_injection/async_utils.py | 3 ++- slack_bolt/kwargs_injection/utils.py | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/slack_bolt/authorization/async_authorize.py b/slack_bolt/authorization/async_authorize.py index 25756a6c4..518c33c24 100644 --- a/slack_bolt/authorization/async_authorize.py +++ b/slack_bolt/authorization/async_authorize.py @@ -73,9 +73,7 @@ async def __call__( if k not in all_available_args: all_available_args[k] = v - kwargs: Dict[str, Any] = { # type: ignore - k: v for k, v in all_available_args.items() if k in self.arg_names # type: ignore - } + kwargs: Dict[str, Any] = all_available_args found_arg_names = kwargs.keys() for name in self.arg_names: if name not in found_arg_names: diff --git a/slack_bolt/authorization/authorize.py b/slack_bolt/authorization/authorize.py index b6f4294aa..20b64182e 100644 --- a/slack_bolt/authorization/authorize.py +++ b/slack_bolt/authorization/authorize.py @@ -77,9 +77,7 @@ def __call__( if k not in all_available_args: all_available_args[k] = v - kwargs: Dict[str, Any] = { # type: ignore - k: v for k, v in all_available_args.items() if k in self.arg_names # type: ignore - } + kwargs: Dict[str, Any] = all_available_args found_arg_names = kwargs.keys() for name in self.arg_names: if name not in found_arg_names: diff --git a/slack_bolt/kwargs_injection/async_utils.py b/slack_bolt/kwargs_injection/async_utils.py index 02da4b651..c4d7fabb9 100644 --- a/slack_bolt/kwargs_injection/async_utils.py +++ b/slack_bolt/kwargs_injection/async_utils.py @@ -94,7 +94,7 @@ def build_async_required_kwargs( # We are sure that we should skip manipulating this arg required_arg_names.pop(0) - kwargs: Dict[str, Any] = {k: v for k, v in all_available_args.items() if k in required_arg_names} + kwargs: Dict[str, Any] = all_available_args found_arg_names = kwargs.keys() for name in required_arg_names: if name == "args": @@ -106,4 +106,5 @@ def build_async_required_kwargs( if name not in found_arg_names: warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning) + kwargs[name] = all_available_args.get(name) return kwargs diff --git a/slack_bolt/kwargs_injection/utils.py b/slack_bolt/kwargs_injection/utils.py index 99d97832d..878dc9699 100644 --- a/slack_bolt/kwargs_injection/utils.py +++ b/slack_bolt/kwargs_injection/utils.py @@ -94,7 +94,7 @@ def build_required_kwargs( # We are sure that we should skip manipulating this arg required_arg_names.pop(0) - kwargs: Dict[str, Any] = {k: v for k, v in all_available_args.items() if k in required_arg_names} + kwargs: Dict[str, Any] = all_available_args found_arg_names = kwargs.keys() for name in required_arg_names: if name == "args":