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

Changed $port param to $url, for more flexible configuration Added read/write timeout options, because client/status hang if service not running. #1

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ValerchikV
Copy link

First of all thank you for your wonderful work! It saved me a lot of time and nerves :)
I propose you to review my pull request with changes i think they will be usefull for you and others.

I found that when only port allowed to configure, its not very good because server bind itself to all IP`s by default wich is not good on prod servers because service become public.
Also by default only tcp is used wich not allow me use other transports.

Other small trouble was that when service not running status and client hangs.
I fixed this by adding timeouts.

Please review my changes and i hope they will be usefull.
Thank you.
V.

Added read/write timeout options, because client/status hang if service
not running.
@davegardnerisme
Copy link
Owner

I think this makes sense. I have literally only just noticed your PR! This is the downside of having over zealous email filters. I shall review!

@ghost ghost assigned davegardnerisme Feb 28, 2013
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.

2 participants