Add listener_runner to context object to enable developers to leverage lazy listeners in middleware #1142
Conversation
…e lazy listeners in middleware
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1142 +/- ##
==========================================
+ Coverage 91.99% 92.02% +0.02%
==========================================
Files 195 195
Lines 6621 6631 +10
==========================================
+ Hits 6091 6102 +11
+ Misses 530 529 -1 ☔ View full report in Codecov by Sentry. |
| @@ -14,6 +14,5 @@ then | |||
| black slack_bolt/ tests/ && \ | |||
| pytest -vv $1 | |||
| else | |||
| black slack_bolt/ tests/ && pytest | |||
| fi | |||
There was a problem hiding this comment.
unrelated minor bug in the script
| @@ -735,7 +735,7 @@ def step( | |||
| elif not isinstance(step, WorkflowStep): | |||
| raise BoltError(f"Invalid step object ({type(step)})") | |||
|
|
|||
| self.use(WorkflowStepMiddleware(step, self.listener_runner)) | |||
| self.use(WorkflowStepMiddleware(step)) | |||
There was a problem hiding this comment.
While steps from apps is no longer supported, this change shows the benefit well
| @@ -13,9 +12,8 @@ | |||
| class AsyncWorkflowStepMiddleware(AsyncMiddleware): | |||
| """Base middleware for step from app specific ones""" | |||
|
|
|||
| def __init__(self, step: AsyncWorkflowStep, listener_runner: AsyncioListenerRunner): | |||
There was a problem hiding this comment.
While this is a public class, I am sure that no developer relies on this inconvenient constructor, and more importantly steps from apps is no longer available for Workflow Builder. Thus, this minor breaking change is okay but others feel we should avoid this, I am happy to revert it.
There was a problem hiding this comment.
It's a good thing to note, in case we get issues filed on the repo as a result. Technically from an API perspective, if I understand correctly, the current version of bolt-python is then the last one to support workflow steps from apps as-is.
| def lazy_listener(self): | ||
| self.lazy_called = True | ||
|
|
||
| def process(self, *, req: BoltRequest, resp: BoltResponse, next: Callable[[], BoltResponse]) -> Optional[BoltResponse]: |
There was a problem hiding this comment.
As you can see, this is not so simple and easy, but for advanced developers who want to take full control (including myself), the enhancement by this PR would be greatly valuable.
There was a problem hiding this comment.
Wow this does provide full control
| @staticmethod | ||
| def _build_lazy_request(request: BoltRequest, lazy_func_name: str) -> BoltRequest: | ||
| copied_request = create_copy(request.to_copyable()) | ||
| copied_request.method = "NONE" |
There was a problem hiding this comment.
mypy detected this property is not defined in the request class and actually the property is never used
| @staticmethod | ||
| def _build_lazy_request(request: AsyncBoltRequest, lazy_func_name: str) -> AsyncBoltRequest: | ||
| copied_request = create_copy(request.to_copyable()) | ||
| copied_request.method = "NONE" |
There was a problem hiding this comment.
mypy detected this property is not defined in the request class and actually the property is never used
| @@ -13,9 +12,8 @@ | |||
| class AsyncWorkflowStepMiddleware(AsyncMiddleware): | |||
| """Base middleware for step from app specific ones""" | |||
|
|
|||
| def __init__(self, step: AsyncWorkflowStep, listener_runner: AsyncioListenerRunner): | |||
There was a problem hiding this comment.
It's a good thing to note, in case we get issues filed on the repo as a result. Technically from an API perspective, if I understand correctly, the current version of bolt-python is then the last one to support workflow steps from apps as-is.
This pull request adds "listener_runner," a foundational module for running listener matchers/middleware and lazy listeners in Bolt apps, to the context object. This enhancement enables third-party app developers to start lazy listeners within global custom middleware. Without it, developers need to manually pass the App's current listener_runner to their global middleware, which is quite inflexible and hard to maintain.
Additionally, our new project (the one our team members should know but we cannot publicly mention yet!) requires this functionality.
To see what the use case looks like, please take a look at the test code in this PR. While the
LazyListenerStartermiddleware in test code is quite simple, the actual use case would involve creating a more complex object that can accept a few functions and dispatch requests to the relevant ones. If none of the functions matches, it would pass the request to other middleware and listeners by callingnext().Category (place an
xin each of the[ ])slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements (place an
xin each[ ])Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.