0

I'm trying to write a pretty basic slackbot, starting by looking up the bot's user_id, which is already harder than I was expecting. The only way I can figure from the documentation is to pull a list of every user and then search that list for my bot user. That already seems kind of bizarre, so if anyone can suggest a better approach, I'm all ears.

But right now what is happening is that it finds the user_id and then proceeds to choke:

Ready to look for REDACTED
<type 'dict'>
2016-06-27 20:22:05.380408 DEBUG REDACTED : ABC123
2016-06-27 20:22:05.381173 DEBUG No user found named REDACTED
2016-06-27 20:22:05.385158 DEBUG ABC123

This is my code:

from slackclient import SlackClient
import re

class Bot(object):

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


    def run(self):
        if self.client.rtm_connect():
            self.user_id = self.whoami(self.client.server.username)
        else:
            print("Connection failed.")

    def whoami(self, username):
        """
        finds the ID for this username.
        """
        all_users = self.client.api_call("users.list")
        print "Ready to look for " + username

        for user in all_users['members']:
            if re.match(username, user['name']):
                print(user['name'] + " : " + user['id'])
                user_id = user['id']
        if (user_id not in locals()):
            print("No user found named " + username) 
        else:
            print(user_id)
        return user_id

And then I call it with:

api_token = 'redacted'
username = 'redacted'

first_bot = Bot(api_token, username)
first_bot.run()

I am trying to figure out why it finds the user and then immediately says "No user found" -- and then prints the user_id -- I expected it to print one or the other.

Amanda
  • 12,099
  • 17
  • 63
  • 91
  • 2
    Argh, what horrible code! Do not use `locals()` for that! First of all to check what you wanted you should write `"user_id" not in locals()` quoting `user_id`, otherwise yuo will get an `UnboundLocalError`. Instead you should `break` out of the loop and use the `else` clause of the `for` loop. – Bakuriu Jun 27 '16 at 20:40
  • At least in python 3 you get `False` if you check for a variable in `locals()` if not passing as a string. `n in locals()` → `False`; `'n' in locals()` → `True` – Luis Jun 27 '16 at 20:43
  • @Bakuriu I think what you mean to say is "`locals()` is not a great way to find out whether or not a variable has been set because {reason}. But you left out the {reason} part. :) At least one question explicitly recommends using `locals()` tho... with the variable name in `'quotes'`. – Amanda Jun 27 '16 at 20:44
  • 2
    @Amanda: `locals()` is a debugging tool, and since locals in functions are optimised, is not even extensible (you can't add more locals this way). – Martijn Pieters Jun 27 '16 at 20:46
  • 1
    @Amanda: The normal way to detect that a loop did not assign anything to a variable is to set the variable to a sentinel before the loop. `user_id = None`, then your loop with `user_id = ...` in an `if` statement, then after the loop: `if user_id is None:` to test if the sentinel is still there. – Martijn Pieters Jun 27 '16 at 20:47
  • Note that your output can't even be produced by the code you posted, because the `self._log("No user found named ")` call doesn't even include `username`. If that line is not matching your output, you can be sure that the *rest* of the code is in doubt too. – Martijn Pieters Jun 27 '16 at 20:48
  • @MartijnPieters that's a typo. I did a fair amount of cutting and pasting as I was formatting the question. Fixed. – Amanda Jun 27 '16 at 21:01
  • @Amanda It's still not possible for both the `if` and `else` clauses to execute. I suspect this code isn't exactly what you're running. – user94559 Jun 27 '16 at 21:14
  • @Amanda: but is the rest of your code still representative? Because apart from the issue that the accepted answer mentions, it *can't produce the output you claim*. Python's `if` statement will only ever execute only *one* of the suites. – Martijn Pieters Jun 27 '16 at 21:14
  • Happy to delete/close if that's preferable to leaving it -- @smarx version works; this was a super helpful lesson in ways to avoid the solution described at http://stackoverflow.com/a/843293/233467 but agreed: I trimmed the code too much and didn't keep testing it before I posted. The only actual error was that I didn't have `user_id` in quotes; both of @smarx solutions are better than fixing the quotes. – Amanda Jun 27 '16 at 21:19

1 Answers1

0

I think if ("user_id" not in locals()): (note the quotes around user_id) would do the trick, but I'd strongly recommend not using locals().

Here's one of many possible fixes:

def whoami(self, username):
    """
    finds the ID for this username.
    """
    all_users = self.client.api_call("users.list")
    print "Ready to look for " + username

    user_id = None

    for user in all_users['members']:
        if re.match(username, user['name']):
            self._log(user['name'] + " : " + user['id'])
            user_id = user['id']

    if user_id is None:
        self._log("No user found named ") 

    return user_id

EDIT

Also, completely untested, but here's a better way to get the bot's user ID:

def whoami(self):
    """
    finds the bot's user ID
    """

    return self.client.api_call("auth.test")["user_id"]
user94559
  • 59,196
  • 6
  • 103
  • 103