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

Sleep for slack #160

Conversation

kmarekspartz
Copy link
Contributor

This addresses the following:

In general we allow applications that integrate with Slack to send no more than one message per second.

From: https://api.slack.com/docs/rate-limits

Thanks, @katherinelim!

This does not fix #132 completely, as we're still making more requests than necessary, but should mean we do not have to do the rate limit sleeps.

@coveralls
Copy link

coveralls commented Nov 3, 2017

Coverage Status

Coverage decreased (-0.2%) to 70.53% when pulling 76d4981 on kmarekspartz:sleep-1-before-requests-to-slack into d8281fa on randsleadershipslack:master.

@coveralls
Copy link

coveralls commented Nov 3, 2017

Coverage Status

Coverage decreased (-0.2%) to 70.53% when pulling 386118a on kmarekspartz:sleep-1-before-requests-to-slack into d8281fa on randsleadershipslack:master.

@kmarekspartz
Copy link
Contributor Author

Unfortunately, when I tested this, it took almost twice as long. I'm thinking about putting this behind an environment flag for now, so that @katherinelim can enable it, then doing #132, then turn this on by default.

@ChipWolf
Copy link

ChipWolf commented Feb 20, 2019

We've pulled this into our version in order to ensure compliance with Slack's ToS, excluding line 63 which isn't necessary and should resolve @kmarekspartz's speed concern

@kmarekspartz
Copy link
Contributor Author

Closing as stale

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

Successfully merging this pull request may close these issues.

Optimize slacker requests
3 participants