Basic call-and-response Slack bot

Question

I'd love thoughts on how to make this more elegant. I'm planning to add some additional functionality, but basically this is just my first pass at creating a pythonic Slack bot from scratch.

You need Slack API token to run it. I am deploying to Heroku, so I have it looking for an environment variable called API_TOKEN which I set (in bash, anyway) export API_TOKEN='abc-1234xyzFakeTokenString'

There are two files, app.py:

from slackclient import SlackClient
import time
import re
import datetime


class Bot(object):

    def __init__(self, token, searches):
        self.searches = searches
        self.client = SlackClient(token)
        self.username = {}
        self.user_id = {}

    def run(self):
        if self.client.rtm_connect():
            try:
                self.user_id = self.client.api_call("auth.test")['user_id']
                self.username = self.client.api_call("auth.test")['user']
                self._log(self.username + ": " + self.user_id)
            except:
                print Exception
            while True:
                self.process_messages(self.client.rtm_read())
                time.sleep(0.25)
        else:
            self._log("Connection failed.")

    def _log(self, message, level=' DEBUG '):
        """
        TODO this should actually put logs somewhere useful;
        """
        print str(datetime.datetime.utcnow()) + level + message

    def filter_speak(self, room, message):
        """
        posts a message to a channel if it matches the call.
        """
        for call in self.searches:
            response = self.searches[call]
            if re.search(call, message):
                self.client.api_call("chat.postMessage", as_user="true",
                                     channel=room, text=response)

    def process_messages(self, messages):
        for msg in messages:
            # We're only interested in entries of type "message"
            if msg['type'] == "message":
                # TODO also check the text of expanded links.
                if 'text' in msg:
                    body = msg['text']
                elif 'subtype' in msg:
                    if msg['subtype'] == "message_changed":
                        body = msg['message']['text']
                else:
                    self._log(msg + " didn't appear to have text or subtype?")

                if 'user' in msg:
                    if msg['user'] != self.user_id:
                        self.filter_speak(message=body, room=msg['channel'])

And then in a separate run.py file I have:

from app import Bot
from os import environ

pairs = {}

pairs["[kK]nock[, -]*[kK]nock"] = "Who's there?"

pope_bot = Bot(environ['API_TOKEN'], pairs)
pope_bot.run()

I'm hoping to add a bit more functionality -- I'd like it to chirp a few more things when prompted by a DM, for instance, but this works so I wanted feedback on how I could streamline it or what you might have done differently.

Answer 1

It is almost never a good idea to catch all exceptions in one block. If you are using a try block, there are probably certain types of exceptions that you are expecting. If some exception shows up that you aren't expecting, you want to know about it. It should be thrown -- stop the program -- let's deal with this. Make a list of the exceptions that you want to catch. Add them to your except line with parentheses around them and commas between them. If you want to print the exception, add as e: to the end and put print e inside the block. Doing print Exception means this is your output:

<type 'exceptions.Exception'>

I don't think that is what you want.

---

Getting key-value pairs in a dictionary is so common that Python made it easy. You can use something like for call, response in self.searches.items(): in filter_speak(). Since you are using Python 2, it would be more memory-efficient to use .iteritems() (In Python 3, .iteritems() was removed and .items() changed to take its place.)

---

You seem to have a bug. What if msg does not have the text key, does have the subtype key, but msg['subtype'] does not equal message_changed? What if msg also has the user key and msg['user'] does not equal self.user_id? Wheew, that takes a lot. If all of that happens, your program will have a NameError because you never define body in that first sentence, but it is required in the second. You need to combine the elif and the if inside it:

if msg['type'] == "message":
    if 'text' in msg:
        body = msg['text']
    elif msg.get('subtype') == "message_changed":
        body = msg['message']['text']
    else:
        ...

Your later if statements can be combined:

if msg.get('user') != self.user_id:
    ...

Ah, the wonderful advantage of dict.get().