Skip to content

Commit

Permalink
Merge pull request #662 from seratch/new-sync-mode
Browse files Browse the repository at this point in the history
Revised sync mode WebClient/RTMClient to address concurrency issues
  • Loading branch information
seratch committed May 15, 2020
2 parents af79b19 + 227f949 commit 58134fe
Show file tree
Hide file tree
Showing 21 changed files with 1,109 additions and 651 deletions.
14 changes: 7 additions & 7 deletions integration_tests/rtm/test_issue_530.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
import logging
import unittest

import pytest

from integration_tests.helpers import async_test, is_not_specified
from integration_tests.helpers import async_test
from slack import RTMClient


Expand All @@ -22,27 +20,29 @@ def tearDown(self):
# Reset the decorators by @RTMClient.run_on
RTMClient._callbacks = collections.defaultdict(list)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
def test_issue_530(self):
try:
rtm_client = RTMClient(token="I am not a token", run_async=False, loop=asyncio.new_event_loop())
rtm_client.start()
self.fail("Raising an error here was expected")
except Exception as e:
self.assertEqual(str(e), "The server responded with: {'ok': False, 'error': 'invalid_auth'}")
self.assertEqual(
"The request to the Slack API failed.\n"
"The server responded with: {'ok': False, 'error': 'invalid_auth'}", str(e))
finally:
if not rtm_client._stopped:
rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@async_test
async def test_issue_530_async(self):
try:
rtm_client = RTMClient(token="I am not a token", run_async=True)
await rtm_client.start()
self.fail("Raising an error here was expected")
except Exception as e:
self.assertEqual(str(e), "The server responded with: {'ok': False, 'error': 'invalid_auth'}")
self.assertEqual(
"The request to the Slack API failed.\n"
"The server responded with: {'ok': False, 'error': 'invalid_auth'}", str(e))
finally:
if not rtm_client._stopped:
rtm_client.stop()
Expand Down
15 changes: 6 additions & 9 deletions integration_tests/rtm/test_issue_569.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,20 @@ def run_cpu_monitor(self):
TestRTMClient.cpu_monitor.setDaemon(True)
TestRTMClient.cpu_monitor.start()

self.rtm_client = None
self.web_client = None

def tearDown(self):
# Reset the decorators by @RTMClient.run_on
RTMClient._callbacks = collections.defaultdict(list)
# Stop the Client
if hasattr(self, "rtm_client") and not self.rtm_client._stopped:
self.rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@pytest.mark.skipif(condition=is_not_specified(), reason="To avoid rate_limited errors")
def test_cpu_usage(self):
self.rtm_client = RTMClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop())
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop())
self.rtm_client = RTMClient(token=self.bot_token, run_async=False, loop=asyncio.new_event_loop())
self.web_client = WebClient(token=self.bot_token, run_async=False)

self.call_count = 0
TestRTMClient.cpu_usage = 0
Expand Down
4 changes: 1 addition & 3 deletions integration_tests/rtm/test_issue_605.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import asyncio
import collections
import logging
import os
Expand Down Expand Up @@ -31,7 +30,7 @@ def tearDown(self):
# Reset the decorators by @RTMClient.run_on
RTMClient._callbacks = collections.defaultdict(list)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@pytest.mark.skipif(condition=is_not_specified(), reason="To avoid rate_limited errors")
def test_issue_605(self):
self.text = "This message was sent to verify issue #605"
self.called = False
Expand All @@ -56,7 +55,6 @@ def connect():
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)
new_message = self.web_client.chat_postMessage(channel=self.channel_id, text=self.text)
self.assertFalse("error" in new_message)
Expand Down
12 changes: 5 additions & 7 deletions integration_tests/rtm/test_issue_631.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ def tearDown(self):
if hasattr(self, "rtm_client") and not self.rtm_client._stopped:
self.rtm_client.stop()

@pytest.mark.skipif(condition=is_not_specified(), reason="this is just for reference")
@pytest.mark.skipif(condition=is_not_specified(), reason="to avoid rate_limited errors")
def test_issue_631_sharing_event_loop(self):
self.success = None
self.text = "This message was sent to verify issue #631"

self.rtm_client = RTMClient(
token=self.bot_token,
run_async=False, # even though run_async=False, handlers for RTM events can be a coroutine
loop=asyncio.new_event_loop(), # TODO: remove this
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)

# @RTMClient.run_on(event="message")
Expand Down Expand Up @@ -72,8 +72,7 @@ def test_issue_631_sharing_event_loop(self):

# Solution (1) for #631
@RTMClient.run_on(event="message")
# even though run_async=False, handlers for RTM events can be a coroutine
async def send_reply(**payload):
def send_reply(**payload):
self.logger.debug(payload)
data = payload['data']
web_client = payload['web_client']
Expand All @@ -82,7 +81,7 @@ async def send_reply(**payload):
if "text" in data and self.text in data["text"]:
channel_id = data['channel']
thread_ts = data['ts']
self.success = await web_client.chat_postMessage(
self.success = web_client.chat_postMessage(
channel=channel_id,
text="Thanks!",
thread_ts=thread_ts
Expand All @@ -106,7 +105,6 @@ def connect():
self.web_client = WebClient(
token=self.bot_token,
run_async=False,
loop=asyncio.new_event_loop(), # TODO: this doesn't work without this
)
new_message = self.web_client.chat_postMessage(channel=self.channel_id, text=self.text)
self.assertFalse("error" in new_message)
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/samples/issues/issue_497.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
)


# This doesn't work
# Fixed in 2.6.0: This doesn't work
@app.route("/sync/singleton", methods=["GET"])
def singleton():
try:
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/web/test_issue_378.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_issue_378(self):
self.assertIsNotNone(response)

@async_test
async def test_issue_378(self):
async def test_issue_378_async(self):
client = self.async_client
response = await client.users_setPhoto(image="tests/data/slack_logo_new.png")
self.assertIsNotNone(response)
1 change: 0 additions & 1 deletion integration_tests/web/test_issue_480.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,3 @@ async def test_issue_480_threads_async(self):
self.assertIsNotNone(response)
after = threading.active_count()
self.assertEqual(0, after - before)

5 changes: 2 additions & 3 deletions integration_tests/web/test_issue_560.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import asyncio
import logging
import os
import unittest
Expand All @@ -17,7 +16,7 @@ class TestWebClient(unittest.TestCase):
def setUp(self):
self.logger = logging.getLogger(__name__)
self.bot_token = os.environ[SLACK_SDK_TEST_BOT_TOKEN]
self.sync_client: WebClient = WebClient(token=self.bot_token, run_async=False, loop=asyncio.new_event_loop())
self.sync_client: WebClient = WebClient(token=self.bot_token, run_async=False)
self.async_client: WebClient = WebClient(token=self.bot_token, run_async=True)

def tearDown(self):
Expand All @@ -37,7 +36,7 @@ async def test_issue_560_success_async(self):
response = await client.conversations_list(exclude_archived=1)
self.assertIsNotNone(response)

response = client.conversations_list(exclude_archived="true")
response = await client.conversations_list(exclude_archived="true")
self.assertIsNotNone(response)

def test_issue_560_failure(self):
Expand Down
29 changes: 29 additions & 0 deletions integration_tests/web/test_web_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,19 @@ def test_uploading_binary_files(self):
deletion = client.files_delete(file=upload["file"]["id"])
self.assertIsNotNone(deletion)

def test_uploading_binary_files_as_content(self):
client = self.sync_client
current_dir = os.path.dirname(__file__)
file = f"{current_dir}/../../tests/data/slack_logo.png"
with open(file, 'rb') as f:
content = f.read()
upload = client.files_upload(
channels=self.channel_id, title="Good Old Slack Logo", filename="slack_logo.png", content=content)
self.assertIsNotNone(upload)

deletion = client.files_delete(file=upload["file"]["id"])
self.assertIsNotNone(deletion)

@async_test
async def test_uploading_binary_files_async(self):
client = self.async_client
Expand All @@ -221,6 +234,22 @@ def test_pagination_with_iterator(self):

self.assertGreater(fetched_count, 1)

def test_pagination_with_iterator_use_sync_aiohttp(self):
client: WebClient = WebClient(
token=self.bot_token,
run_async=False,
use_sync_aiohttp=True,
loop=asyncio.new_event_loop(),
)
fetched_count = 0
# SlackResponse is an iterator that fetches next if next_cursor is not ""
for response in client.conversations_list(limit=1, exclude_archived=1, types="public_channel"):
fetched_count += len(response["channels"])
if fetched_count > 1:
break

self.assertGreater(fetched_count, 1)

@pytest.mark.skipif(condition=is_not_specified(), reason="still unfixed")
@async_test
async def test_pagination_with_iterator_async(self):
Expand Down
Loading

0 comments on commit 58134fe

Please sign in to comment.