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

Stable acknowledgement for incoming requests #84

Closed
5 of 9 tasks
seratch opened this issue Dec 2, 2020 · 1 comment
Closed
5 of 9 tasks

Stable acknowledgement for incoming requests #84

seratch opened this issue Dec 2, 2020 · 1 comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality

Comments

@seratch
Copy link
Member

seratch commented Dec 2, 2020

Description

This library runs an event listener before responding to an incoming request from Slack with 200 OK. This means if an event handler takes over 3 seconds, the event request times out and the Slack API server resends the same event up to 3 times. You can easily reproduce this issue with the following code snippet.

import time
@slack_events_adapter.on("app_mention")
def handle_app_mentions(event_data):
    time.sleep(5)
    print("Done!")

As a workaround, developers can use threads to run time-consuming tasks asynchronously. Mixing asyncio may not be a great idea as this is a Flask extension and Flask does not support the mechanism.

import time
from concurrent.futures.thread import ThreadPoolExecutor
executor = ThreadPoolExecutor(5)
@slack_events_adapter.on("app_mention")
def handle_app_mentions(event_data):
    def do_something():
        time.sleep(5)
        print("Done!")
    executor.submit(do_something)
    print("Accepted!")

Improving this library is of course worth considering but in the short run, it's not a priority. Also, not only due to this matter, we recommend using Bolt for Python. The new library supports not only Events API but also all the latest features including interactivity.

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.
@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented enhancement M-T: A feature request for new functionality discussion M-T: An issue where more input is needed to reach a decision labels Dec 2, 2020
@seratch seratch added this to the 2.3.0 milestone Dec 2, 2020
@seratch seratch removed this from the 3.0.0 milestone Jun 2, 2021
@seratch
Copy link
Member Author

seratch commented Jun 2, 2021

I'm sorry to say this, but we are not going to implement this feature.

If you build a Slack app with time-consuming event listeners, we recommend using Bolt for Python instead. The novel framework has great consideration of 3 second timeouts out-of-the-box. You can call ack() method first and then do whatever takes longer than 3 seconds in the same method (under the hood, it uses threads). In addition to that, Bolt has a built-in adapter for Flask. You can find working examples here: https://github.com/slackapi/bolt-python/tree/main/examples/flask

I hope developers in the community will like Bolt too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality
Projects
None yet
Development

No branches or pull requests

1 participant