Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add limits #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add limits #1

wants to merge 1 commit into from

Conversation

hildjj
Copy link

@hildjj hildjj commented Jan 9, 2015

After Ryan said "hubot pug bomb 400', none of us can have nice things anymore. Default behavior is unchanged, but add HUBOT_PUGBOMB_MAX=[number] to keep Ryan from breaking Slack again.

@devinreams
Copy link

I only saw 20 come through for what it's worth...

👍

@hildjj
Copy link
Author

hildjj commented Jan 9, 2015

If he would have used the correct syntax it would have been catastrophic. I'm proactively blocking his intent.

@J3RN
Copy link

J3RN commented Apr 8, 2015

👍 Someone in an IRC channel I'm in tried to do 2000. It crashed after ~700.

@KyleJamesWalker
Copy link

Would love a limit! Otherwise I've just bee disabling the script or replacing with:

# Description:
#   Pugme is the most important thing in your life
#
# Dependencies:
#   None
#
# Configuration:
#   HUBOT_PUGME_MAX - The most pugs you can bomb.  Default: 20
#
# Commands:
#   hubot pug me - Receive a pug
#   hubot pug bomb N - get N pugs

module.exports = (robot) ->
  max = (process.env.HUBOT_PUGME_MAX * 1) or 20

  robot.respond /pug me/i, (msg) ->
    msg.http("http://pugme.herokuapp.com/random")
      .get() (err, res, body) ->
        msg.send JSON.parse(body).pug

  robot.respond /pug bomb( (\d+))?/i, (msg) ->
    count = msg.match[2] || 5
    count = max if count > max

    msg.http("http://pugme.herokuapp.com/bomb?count=" + count)
      .get() (err, res, body) ->
        msg.send pug for pug in JSON.parse(body).pugs

  robot.respond /how many pugs are there/i, (msg) ->
    msg.http("http://pugme.herokuapp.com/count")
      .get() (err, res, body) ->
        msg.send "There are #{JSON.parse(body).pug_count} pugs."

I like the idea of some pugs still making it through when someone tries inserting a crazy number.

Note slack boots you right around a bomb of 17 pugs with:

ERROR Received error {"code":-1,"msg":"slow down, too many messages..."}

@technicalpickles
Copy link
Contributor

Sorry I missed this, wasn't watching the repo.

If we are adding a limit, we may as well have a reasonable one, like 5 or 10, rather than unlimited.

@brgaulin
Copy link

👍 is there any reason to not merge this? Does it need a test or anything? We would love to set a limit for our hubot

@gep13
Copy link

gep13 commented Nov 9, 2015

@technicalpickles is this something you are planning on pulling in? We have just ran into the same issue with a couple of our bots. Thanks!

@ferventcoder
Copy link

I agree with a reasonable limit to start off with. :)

gep13 added a commit to cake-build/bot that referenced this pull request Nov 10, 2015
- Due to this:
https://gitter.im/cake-build/cake?at=56410e2d8b872257348e17ef
- If/when this PR gets accepted:
hubot-archive/hubot-pugme#1
- We can think about adding this back in
gep13 added a commit to GitTools/gitbot-gitter that referenced this pull request Nov 10, 2015
- Due to this:
https://gitter.im/cake-build/cake?at=56410e2d8b872257348e17ef
- If/when this PR gets accepted:
hubot-archive/hubot-pugme#1
- We can think about adding this back in
gep13 added a commit to chocolatey-community/chocobot-gitter that referenced this pull request Nov 10, 2015
- Due to this:
https://gitter.im/cake-build/cake?at=56410e2d8b872257348e17ef
- If/when this PR gets accepted:
hubot-archive/hubot-pugme#1
- We can think about adding this back in
@hmemcpy
Copy link

hmemcpy commented Nov 10, 2015

As a pug, NOOOOOOOOOOOOOOOOOOOOOO!!!! :((

@hmemcpy
Copy link

hmemcpy commented Nov 10, 2015

image

#
# Commands:
# hubot pug me - Receive a pug
# hubot pug bomb N - get N pugs

module.exports = (robot) ->
max = (process.env.HUBOT_PUGBOMB_MAX*1) or -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found recently you can parseInt an undefined and get NaN back, and that's considered falsy. That means you can parseInt(process.env.HUBOT_PUGBOMB_MAX) or -1.

@technicalpickles
Copy link
Contributor

is there any reason to not merge this?

I'm looking for a reasonable default to the limit, rather than unlimited by default.

@brgaulin
Copy link

brgaulin commented Jan 9, 2016

5 is what I normally use.
On Fri, Jan 8, 2016 at 7:51 PM Josh Nichols [email protected]
wrote:

is there any reason to not merge this?

I'm looking for a reasonable default to the limit, rather than unlimited
by default.


Reply to this email directly or view it on GitHub
#1 (comment)
.

Brendan Gaulin

@gep13
Copy link

gep13 commented Jan 9, 2016

@technicalpickles said...
I'm looking for a reasonable default to the limit, rather than unlimited by default.

Surely, any arbirtrary number, is better than having it be unlimited!?!

I would also suggest something like 5 as the default, and perhaps provide the ability to configure this via an environment variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

9 participants