add Slack error handling and remove use of deprecated API method (#141)

* fix: deprecated api methods and error handling

* fix test

* add README

* minor fix

* Update gokart/slack/README.md

apply review suggestion

Co-authored-by: K.O. <hikingko1@gmail.com>

* add slack_notification document on doc directory

Co-authored-by: hirohito-sasakawa <hirohito-sasakawa@m3.com>
Co-authored-by: K.O. <hikingko1@gmail.com>

Author: 3 people

docs/index.rst

@@ -20,7 +20,7 @@ Table of Contents
    task_on_kart
    task_information
    task_settings
-
+   slack_notification
 
 API References
 --------------

docs/slack_notification.rst

@@ -0,0 +1,37 @@
+gokart Slack notification
+=========================
+
+Prerequisites
+-------------
+
+Prepare following environmental variables:
+
+.. code:: sh
+
+   export SLACK_TOKEN=xoxb-your-token    // should use token starts with "xoxb-" (bot token is preferable)
+   export SLACK_CHANNEL=channel-name     // not "#channel-name", just "channel-name"
+
+
+A Slack bot token can obtain from `here <https://api.slack.com/apps>`_.
+
+A bot token needs following scopes:
+
+- `channels:read`
+- `chat:write`
+- `files:write`
+
+More about scopes are `here <https://api.slack.com/scopes>`_.
+
+Implement Slack notification
+----------------------------
+
+Write following codes pass arguments to your gokart workflow.
+
+.. code:: python
+    cmdline_args = sys.argv[1:]
+    if 'SLACK_CHANNEL' in os.environ:
+        cmdline_args.append(f'--SlackConfig-channel={os.environ["SLACK_CHANNEL"]}')
+    if 'SLACK_TO_USER' in os.environ:
+        cmdline_args.append(f'--SlackConfig-to-user={os.environ["SLACK_TO_USER"]}')
+    gokart.run(cmdline_args)
+

gokart/slack/slack_api.py

@@ -1,7 +1,8 @@
 from logging import getLogger
+import time
 
 import slack
-
+from slack.errors import SlackApiError
 
 logger = getLogger(__name__)
 
@@ -24,33 +25,27 @@ def __init__(self, token, channel: str, to_user: str) -> None:
         self._channel_id = self._get_channel_id(channel)
         self._to_user = to_user if to_user == '' or to_user.startswith('@') else '@' + to_user
 
-    def _get_channels(self, channels=[], cursor=None):
-        params = {}
-        if cursor:
-            params['cursor'] = cursor
-        response = self._client.api_call('channels.list', http_verb="GET", params=params)
-        if not response['ok']:
-            raise ChannelListNotLoadedError(f'Error while loading channels. The error reason is "{response["error"]}".')
-        channels += response.get('channels', [])
-        if not channels:
-            raise ChannelListNotLoadedError('Channel list is empty.')
-        if response['response_metadata']['next_cursor']:
-            return self._get_channels(channels, response['response_metadata']['next_cursor'])
-        else:
-            return channels
-
     def _get_channel_id(self, channel_name):
-        for channel in self._get_channels():
-            if channel['name'] == channel_name:
-                return channel['id']
-        raise ChannelNotFoundError(f'Channel {channel_name} is not found in public channels.')
+        params = {'exclude_archived': True, 'limit': 100}
+        try:
+            for channels in self._client.conversations_list(params=params):
+                if not channels:
+                    raise ChannelListNotLoadedError('Channel list is empty.')
+                for channel in channels.get('channels', []):
+                    if channel['name'] == channel_name:
+                        return channel['id']
+            raise ChannelNotFoundError(f'Channel {channel_name} is not found in public channels.')
+        except (ChannelNotFoundError, SlackApiError) as e:
+            logger.warning(f'The job will start without slack notification: {e}')
 
     def send_snippet(self, comment, title, content):
-        request_body = dict(
-            channels=self._channel_id,
-            initial_comment=f'<{self._to_user}> {comment}' if self._to_user else comment,
-            content=content,
-            title=title)
-        response = self._client.api_call('files.upload', data=request_body)
-        if not response['ok']:
-            raise FileNotUploadedError(f'Error while uploading file. The error reason is "{response["error"]}".')
+        try:
+            request_body = dict(channels=self._channel_id,
+                                initial_comment=f'<{self._to_user}> {comment}' if self._to_user else comment,
+                                content=content,
+                                title=title)
+            response = self._client.api_call('files.upload', data=request_body)
+            if not response['ok']:
+                raise FileNotUploadedError(f'Error while uploading file. The error reason is "{response["error"]}".')
+        except (FileNotUploadedError, SlackApiError) as e:
+            logger.warning(f'Failed to send slack notification: {e}')

setup.py

@@ -31,7 +31,7 @@
     license='MIT License',
     packages=find_packages(),
     install_requires=install_requires,
-    tests_require=['moto==1.3.6'],
+    tests_require=['moto==1.3.6', 'testfixtures==6.14.2'],
     test_suite='test',
     classifiers=['Programming Language :: Python :: 3.6'],
 )

test/slack/test_slack_api.py

@@ -3,65 +3,118 @@
 from unittest import mock
 from unittest.mock import MagicMock
 
+from testfixtures import LogCapture
+from slack.web.slack_response import SlackResponse
+from slack import WebClient
+
 import gokart.slack
 
 logger = getLogger(__name__)
 
 
+def _slack_response(token, data):
+    return SlackResponse(client=WebClient(token=token),
+                         http_verb="POST",
+                         api_url="http://localhost:3000/api.test",
+                         req_args={},
+                         data=data,
+                         headers={},
+                         status_code=200)
+
+
 class TestSlackAPI(unittest.TestCase):
     @mock.patch('gokart.slack.slack_api.slack.WebClient')
     def test_initialization_with_invalid_token(self, patch):
-        def _channels_list(method, http_verb="POST", params={}):
-            assert method == 'channels.list'
-            return {'ok': False, 'error': 'error_reason'}
+        def _conversations_list(params={}):
+            return _slack_response(token='invalid', data={'ok': False, 'error': 'error_reason'})
 
         mock_client = MagicMock()
-        mock_client.api_call = MagicMock(side_effect=_channels_list)
+        mock_client.conversations_list = MagicMock(side_effect=_conversations_list)
         patch.return_value = mock_client
 
-        with self.assertRaises(gokart.slack.slack_api.ChannelListNotLoadedError):
+        with LogCapture() as l:
             gokart.slack.SlackAPI(token='invalid', channel='test', to_user='test user')
+            l.check(('gokart.slack.slack_api', 'WARNING',
+                     'The job will start without slack notification: Channel test is not found in public channels.'))
 
     @mock.patch('gokart.slack.slack_api.slack.WebClient')
     def test_invalid_channel(self, patch):
-        def _channels_list(method, http_verb="POST", params={}):
-            assert method == 'channels.list'
-            return {'ok': True, 'channels': [{'name': 'valid', 'id': 'valid_id'}], 'response_metadata': {'next_cursor': ''}}
+        def _conversations_list(params={}):
+            return _slack_response(token='valid',
+                                   data={
+                                       'ok': True,
+                                       'channels': [{
+                                           'name': 'valid',
+                                           'id': 'valid_id'
+                                       }],
+                                       'response_metadata': {
+                                           'next_cursor': ''
+                                       }
+                                   })
 
         mock_client = MagicMock()
-        mock_client.api_call = MagicMock(side_effect=_channels_list)
+        mock_client.conversations_list = MagicMock(side_effect=_conversations_list)
         patch.return_value = mock_client
 
-        with self.assertRaises(gokart.slack.slack_api.ChannelNotFoundError):
+        with LogCapture() as l:
             gokart.slack.SlackAPI(token='valid', channel='invalid_channel', to_user='test user')
+            l.check((
+                'gokart.slack.slack_api', 'WARNING',
+                'The job will start without slack notification: Channel invalid_channel is not found in public channels.'
+            ))
 
     @mock.patch('gokart.slack.slack_api.slack.WebClient')
     def test_send_snippet_with_invalid_token(self, patch):
-        def _api_call(*args, **kwargs):
-            if args[0] == 'channels.list':
-                return {'ok': True, 'channels': [{'name': 'valid', 'id': 'valid_id'}], 'response_metadata': {'next_cursor': ''}}
-            if args[0] == 'files.upload':
-                return {'ok': False, 'error': 'error_reason'}
-            assert False
+        def _conversations_list(params={}):
+            return _slack_response(token='valid',
+                                   data={
+                                       'ok': True,
+                                       'channels': [{
+                                           'name': 'valid',
+                                           'id': 'valid_id'
+                                       }],
+                                       'response_metadata': {
+                                           'next_cursor': ''
+                                       }
+                                   })
+
+        def _api_call(method, data={}):
+            assert method == 'files.upload'
+            return {'ok': False, 'error': 'error_reason'}
 
         mock_client = MagicMock()
+        mock_client.conversations_list = MagicMock(side_effect=_conversations_list)
         mock_client.api_call = MagicMock(side_effect=_api_call)
         patch.return_value = mock_client
 
-        with self.assertRaises(gokart.slack.slack_api.FileNotUploadedError):
+        with LogCapture() as l:
             api = gokart.slack.SlackAPI(token='valid', channel='valid', to_user='test user')
             api.send_snippet(comment='test', title='title', content='content')
+            l.check(
+                ('gokart.slack.slack_api', 'WARNING',
+                 'Failed to send slack notification: Error while uploading file. The error reason is "error_reason".'))
 
     @mock.patch('gokart.slack.slack_api.slack.WebClient')
     def test_send(self, patch):
-        def _api_call(*args, **kwargs):
-            if args[0] == 'channels.list':
-                return {'ok': True, 'channels': [{'name': 'valid', 'id': 'valid_id'}], 'response_metadata': {'next_cursor': ''}}
-            if args[0] == 'files.upload':
-                return {'ok': True}
-            assert False
+        def _conversations_list(params={}):
+            return _slack_response(token='valid',
+                                   data={
+                                       'ok': True,
+                                       'channels': [{
+                                           'name': 'valid',
+                                           'id': 'valid_id'
+                                       }],
+                                       'response_metadata': {
+                                           'next_cursor': ''
+                                       }
+                                   })
+
+        def _api_call(method, data={}):
+            assert method == 'files.upload'
+            return {'ok': False, 'error': 'error_reason'}
 
         mock_client = MagicMock()
+        mock_client.conversations_list = MagicMock(side_effect=_conversations_list)
         mock_client.api_call = MagicMock(side_effect=_api_call)
         patch.return_value = mock_client