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

AsyncIO loop is not being shared from RTMClient to WebClient #631

Closed
4 of 9 tasks
juan-vg opened this issue Mar 4, 2020 · 5 comments
Closed
4 of 9 tasks

AsyncIO loop is not being shared from RTMClient to WebClient #631

juan-vg opened this issue Mar 4, 2020 · 5 comments
Labels
Milestone

Comments

@juan-vg
Copy link

juan-vg commented Mar 4, 2020

Description

Describe your issue here.

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

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

Requirements (place an x in each of the [ ])

  • 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.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

slackclient version: 2.5.0

python version: 3.7.6 (venv)

Steps to reproduce:

  1. Use SlackClient as part of a project, not only as a standalone stuff. The project uses other libraries that use asyncio
  2. Create a RTMClient, and set the loop as asyncio.get_event_loop() in order to use the same as the project
  3. When receiving a message, get the attached web_client and use it to send a message back

Expected result:

The message is sent

Actual result:

RuntimeError: this event loop is already running.

Explanation of what's going on and workaround:

What is actually happening is that the loop set to the RTMClient is not being shared with the attached WebClient.

The following code does the trick, but it's a workaround and not a real solution

import os
from slack import RTMClient
import asyncio
loop = asyncio.get_event_loop()

@RTMClient.run_on(event="message")
def say_hello(**payload):
  data = payload['data']
  web_client = payload['web_client']

#### WORKAROUND #########################
# Manually set the project's loop on the WebClient, even when it was set on the parent RTMClient
  web_client._event_loop = loop
  # Maybe you will also need the following line uncommented
  # web_client.run_async = True
##########################################

  if 'Hello' in data['text']:
    channel_id = data['channel']
    thread_ts = data['ts']
    user = data['user']

    web_client.chat_postMessage(
      channel=channel_id,
      text=f"Hi <@{user}>!",
      thread_ts=thread_ts
    )

slack_token = os.environ["SLACK_API_TOKEN"]
rtm_client = RTMClient(token=slack_token, loop=loop)
rtm_client.start()

Regarding uvloop:

If the main project's loop is not the common one but an uvloop, you will need to make some mods before getting the loop in order to set it on the RTMClient

...
import asyncio
import uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
loop = asyncio.get_event_loop()
...
@seratch
Copy link
Member

seratch commented Apr 20, 2020

FYI: this commit contains a test case reproducing this issue and it shows two possible solutions for it even with slackclient 2.5.0. seratch@6fb4287

In the forthcoming release, the library will address the issue for run_async=False mode (the default). The solution I'm currently thinking as the best is implementing the internals of the WebClient never relies on asyncio's event loop if run_async is False.

The internal modification won't bring any breaking changes to the library users. WebClient just ignores the event loop in the case. So, with future versions, you no longer need to have the necessity to share an event loop as long as you go with the default run_async=False option.

@juan-vg
Copy link
Author

juan-vg commented Apr 20, 2020

On the other hand, for the run_async=True option, the solution is as easy as checking if a loop was set on the RTMClient. If so, add it to the params when creating the WebClient (action the RTMClient does internally).

If you manually create a WebClient, you can set your own loop (as you can do on RTMClient creation)

@juan-vg
Copy link
Author

juan-vg commented Apr 20, 2020

I've updated the demo code to highlight the workaround I've made and need to be solved by doing the proper loop/run_async params passing from RTMClient to WebClient

seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 21, 2020
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 27, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 28, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 28, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 30, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 11, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 13, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
seratch added a commit to seratch/python-slack-sdk that referenced this issue May 14, 2020
* slackapi#530 Fixed by changing _execute_in_thread to be a coroutine
* slackapi#569 Resolved by removing a blocking loop (while future.running())
* slackapi#645 WebClient(run_async=False) no longer depends on asyncio by default
* slackapi#633 WebClient(run_async=False) doesn't internally depend on aiohttp
* slackapi#631 When run_async=True, RTM listner can be a normal function and WebClient is free from the event loop
* slackapi#630 WebClient no longer depends on aiohttp when run_async=False
* slackapi#497 Fixed when run_async=False / can be closed as we don't support run_async=True for this use case (in Flask)
@seratch
Copy link
Member

seratch commented May 15, 2020

Let me close this issue now as #662 resolved this for the run_async=False mode.

@seratch seratch closed this as completed May 15, 2020
@seratch
Copy link
Member

seratch commented May 15, 2020

👋 slackclient 2.6.0rc1 is out. The pre-release version contains fixes for your issue described here.
https://pypi.org/project/slackclient/2.6.0rc1/

One week later from now, we'll be releasing version 2.6.0 to PyPI.

If you have a chance, could you try the release candidate version out and let us know your feedback? Thank you very much for being patient with this issue.

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

No branches or pull requests

3 participants