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

don't mute connection when listener is absent #8

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

Conversation

christian-blades-cb
Copy link

@christian-blades-cb christian-blades-cb commented Sep 14, 2016

Corrected tests for #7 to account for expected new behavior.

Fixes #6

zstyblik and others added 2 commits June 8, 2016 11:11
Commit removes "quick listener check" which mutes connection and returns error
when listener isn't available at the time of inicialization.
It seems as this there is no need to scrap connection, because when remote
endpoint comes around later, metrics will be received by that endpoint just
fine.

Fixes alexcesaro#6
* non-listening ports should not cause statsd client to mute
@christian-blades-cb
Copy link
Author

@alexcesaro @zstyblik Hopefully a version with passing tests is more palatable. Or at least can spark a conversation.

I also would like to remove launch order dependencies. Having the stats emitted and available when the collector finally comes up is the behavior I would expect.

@zstyblik
Copy link

@christian-blades-cb, cool :)

@vchernoy
Copy link

Any estimation when to expect the PR to be merged?
This functionality is desired.
Thank you.

@christian-blades-cb
Copy link
Author

@vchernoy Given that @alexcesaro hasn't pushed any commits since April, it's probably time to either fork this repo or move on to a different one. I chose cactus/go-statsd-client for my recent production work.

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.

3 participants