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

[Feature Request] Multiple Listener Dispatching #789

Closed
2 of 4 tasks
WilliamBergamin opened this issue Dec 15, 2022 · 3 comments
Closed
2 of 4 tasks

[Feature Request] Multiple Listener Dispatching #789

WilliamBergamin opened this issue Dec 15, 2022 · 3 comments

Comments

@WilliamBergamin
Copy link
Contributor

WilliamBergamin commented Dec 15, 2022

This is a proposal to allow Bolt to support triggering multiple functions for a common listener, below is an example of what this could look like

@app.event({"type": "message"})
def tag_message():
    # finds and associates tags with the current channel
     
@app.event({"type": "message"})
def activity_monitor():
    # increments activity of the user sending the message

....

Currently the second function is never called.

This was originally brought up in #786

Lets open a discussing to figure out if this is in the scope of Bolt, and wether this should be included in the project

Category

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Others

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch
Copy link
Member

seratch commented Dec 16, 2022

Thanks for starting this discussion!

First off, we should not change the current behavior without any opt-in configuration in any case. If we change the behavior in a future version, the behavior change may cause disruption/data corruption (due to extra code execution) for existing apps.

Thinking of the possibility of this enhancement, I myself am reluctant to make the change to bolt-python. Indeed, bolt-js behaves in the way described in this issue's description. However, I didn't intentionally follow the design when building bolt-java and bolt-python.

Here are the reasons behind my decision back then:

  • As a listener is responsible for calling ack() to acknowledge a request from Slack, calling multiple listeners for a single request does not make sense. Also, the behavior can be confusing in the case of large/complex app development. bolt-js checks if there is no duplicated ack() execution in user-side code. But I believe the risk of such duplication should be avoided on the framework side.
  • If an app has to check all the listeners for a request even after running the first matched one, executing all the listener middleware can cause performance issues when any of them takes long. To make matters worse, if the framework goes with the design, there is no workaround.
  • In my observation, allowing having exactly the same patterns in a framework's routing mechanism is uncommon. When you have two app.get("/foo") listeners in a web app, do you expect both are executed? I personally don't expect such behavior.
  • Lastly, if a developer would like to run multiple operations for a request pattern, I'd suggest going with a big global middleware for it like the way we did for Steps from Apps. It's much more straight-forward for the use case.

I hope this was helpful in understanding the context and the beauty of bolt-python's current design. In the long run, future maintainers may desire to add a new option that enables developers to turn multiple-listener execution on. The implementation can be very complicated, so I don't recommend it. With that being said, as long as the enhancement does not break anything for existing apps, it may be worth exploring.

@github-actions
Copy link

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

@github-actions
Copy link

As this issue has been inactive for more than one month, we will be closing it. Thank you to all the participants! If you would like to raise a related issue, please create a new issue which includes your specific details and references this issue number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants