Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #709 Enable developers to inject non-bolt arguments to listener function args #712

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions slack_bolt/app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

# -------------------------
Expand Down
9 changes: 7 additions & 2 deletions slack_bolt/app/async_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

# -------------------------
Expand Down
7 changes: 5 additions & 2 deletions slack_bolt/authorization/async_authorize.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
from logging import Logger
from typing import Optional, Callable, Awaitable, Dict, Any

Expand All @@ -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:
Expand Down Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow developers to inject extra args, we remove this logic

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:
Expand Down
7 changes: 5 additions & 2 deletions slack_bolt/authorization/authorize.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
from logging import Logger
from typing import Optional, Callable, Dict, Any

Expand All @@ -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:
Expand Down Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow developers to inject extra args, we remove this logic

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:
Expand Down
9 changes: 6 additions & 3 deletions slack_bolt/kwargs_injection/async_utils.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,6 +18,7 @@
to_step,
)
from ..logger.messages import warning_skip_uncommon_arg_name
from ..warning import BoltCodeWarning


def build_async_required_kwargs(
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow developers to inject extra args, we remove this logic

warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning)
return kwargs
9 changes: 6 additions & 3 deletions slack_bolt/kwargs_injection/utils.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -17,6 +18,7 @@
to_step,
)
from ..logger.messages import warning_skip_uncommon_arg_name
from ..warning import BoltCodeWarning


def build_required_kwargs(
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow developers to inject extra args, we remove this logic

warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning)
return kwargs
11 changes: 11 additions & 0 deletions slack_bolt/warning/__init__.py
Original file line number Diff line number Diff line change
@@ -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.
"""