I understand that #381 was done with good intent but the Slack team I run this bot in has more than 10,000 users and putting them all in the brain crosses the limit of any free-tier Redis backend on Heroku because it takes up more than 30MB. I'm not willing to pay for more memory until I can prove the bot's usefulness as proof of concept and the MongoDB brain I tried didn't work at all. For now I've downgraded from 4.4.0 to 4.2.2, before that PR was merged, so that I can have a persistent brain for other non-user things.
Obviously this is a needed feature for some, since it was re-implemented at all, so it can't just be turned off, but maybe saving user data could be made either toggleable or filterable. The toggleability might be the simpler of the two: If the user sets an env var like NO_LOAD_USERS the code path that loads the users to the brain is not run. As a more flexible solution, since in my specific case I know that only ~100 of the ~10,000 users in the team will use this bot at first, it might be worth being able to turn on loading the users to the brain in such a way that the users loaded are filterable by some kind of grouping or categorisation, e.g. they all belong to a named Slack channel.
If we can agree on which of these solutions would be preferable, or someone else has an even better idea, maybe I could try and do a pull request for it because it would be nice to use v4.4.0 without my Redis exploding. :stuck_out_tongue_closed_eyes:
@sinisterstuf wow, i totally understand your frustration.
i like your idea for the environment variable flag. i think it should just disable the loading of users on connect (which loads the entire team). this would still mean that users who interact with hubot are potentially loaded into the brain (if hubot hears a message sent by the user or if the user was mentioned in a message it hears). does that sound reasonable to you?
i'd by happy to merge a PR, but you should probably wait off a bit on development since #465 will touch quite a bit of the code. OR if you're really eager, try to base your work off the branch in that PR.
for context, we're hoping to land that very soon (probably within a day) and have a handful of small changes for the next week or so, and make our next release the week after that.
OK, thanks for the advice. So I see #465 just got merged, so I'll try to do a PR for this. I'm not familiar with hubot-slack internals but I'd assume it's a fairly trivial change.
@sinisterstuf awesome!
i'd recommend looking at slack.coffee in the root directory for reading environment variables and passing them as configuration to the SlackBot (Adapter) and potentially SlackClient objects.
you likely want to just put conditions around the loadUsers call(s)
Cool, just done the PR, it's more or less what you said: #471
Data store can be disabled by running hubot with
HUBOT_SLACK_RTM_CLIENT_OPTS='{"useRtmConnect":true,"dataStore":false}' hubot --adapter slack
but on v4.4.0 this blows up
root@aaec15542349:/opt/apps/hubot# HUBOT_SLACK_RTM_CLIENT_OPTS='{"useRtmConnect":true,"dataStore":false}' hubot --adapter slack
error: TypeError: Cannot read property 'length' of undefined
at SlackBot.authenticated (/opt/apps/hubot/node_modules/hubot-slack/src/bot.coffee:92:5)
at RTMClient.<anonymous> (/opt/apps/hubot/node_modules/hubot-slack/src/bot.coffee:1:1)
at RTMClient.emit (/opt/apps/hubot/node_modules/eventemitter3/index.js:116:35)
at RTMClient._onStart (/opt/apps/hubot/node_modules/@slack/client/lib/clients/rtm/client.js:337:10)
I think maybe all that's needed is a check for the presence of the user list, as @aoberoi pointed out.
Most helpful comment
OK, thanks for the advice. So I see #465 just got merged, so I'll try to do a PR for this. I'm not familiar with hubot-slack internals but I'd assume it's a fairly trivial change.