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

transports/rabbitmq: add timeout option (updated) #345

Merged
merged 4 commits into from Sep 11, 2015
Merged

transports/rabbitmq: add timeout option (updated) #345

merged 4 commits into from Sep 11, 2015

Conversation

timstoop
Copy link
Contributor

This is a duplicate of pull request #253, but updated to be mergable with master again. It adds a timeout option to rabbitmq, since pika defaults to .25 seconds, which can lead to problems when a lot of clients are connecting at the same time.

This solves a practical problem for us, where hundreds of clients can simply overwhelm a rabbitmq server at connection time, so it cannot respond within the .25 seconds. This patch increases the timeout to 2 seconds, which solves the problem for us. It also makes it a setting.

@kitchen
Copy link
Contributor

kitchen commented Sep 10, 2015

My vote: leave the default but make it configurable.

-Jeremy

Insert PGP signature here.

On Sep 10, 2015, at 9:22 AM, Tim Stoop notifications@github.com wrote:

This is a duplicate of pull request #253, but updated to be mergable with master again. It adds a timeout option to rabbitmq, since pika defaults to .25 seconds, which can lead to problems when a lot of clients are connecting at the same time.

This solves a practical problem for us, where hundreds of clients can simply overwhelm a rabbitmq server at connection time, so it cannot respond within the .25 seconds. This patch increases the timeout to 2 seconds, which solves the problem for us. It also makes it a setting.

You can view, comment on, or merge this pull request online at:

#345

Commit Summary

Pieter's patch for rabbitmq timeout.
File Changes

M beaver/config.py (2)
M beaver/transports/rabbitmq_transport.py (5)
M docs/user/usage.rst (1)
Patch Links:

https://github.com/josegonzalez/python-beaver/pull/345.patch
https://github.com/josegonzalez/python-beaver/pull/345.diff

Reply to this email directly or view it on GitHub.

@josegonzalez
Copy link
Member

Can you look into the failing tests? I'll buy you a beer :) (or hot chocolate, if that's your thing).

Apparantly, it needs to be an integer, so we cannot use pika's default
of .25.
@timstoop
Copy link
Contributor Author

Settings need to be integers, apparently. So instead of trying to change that, I decided on a middle ground with a 1 second default. Hope that alright?

And beer is totally great! However, credit should go to @pieterlexis, who actually created the patch. I only made it apply again.

josegonzalez added a commit that referenced this pull request Sep 11, 2015
transports/rabbitmq: add timeout option (updated)
@josegonzalez josegonzalez merged commit 19a675a into python-beaver:master Sep 11, 2015
@timstoop timstoop deleted the rabbitmq_timeout branch September 11, 2015 14:47
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.

None yet

3 participants