Skip to content

Specify bucket prefix on commandline#1

Open
jforest wants to merge 6 commits into
vimeo:masterfrom
CashStar:specify_bucket_prefix_on_commandline
Open

Specify bucket prefix on commandline#1
jforest wants to merge 6 commits into
vimeo:masterfrom
CashStar:specify_bucket_prefix_on_commandline

Conversation

@jforest

@jforest jforest commented Oct 29, 2014

Copy link
Copy Markdown

This modification allows you to specify the bucket to send stats to on the command line, useful for people like me who don't like the default!

@Dieterbe

Copy link
Copy Markdown
Contributor

the indentation got messed up by your editor or something.
can you run gofmt -w smoketcp.go and commit the result, that should show only the real differences.

@jforest

jforest commented Oct 29, 2014

Copy link
Copy Markdown
Author

Fixed!

@Dieterbe

Copy link
Copy Markdown
Contributor

i'm confused. this makes the prefix configurable, but yet it automatically adds "smokeping" to the metric name. also why smokeping, its not a ping test and the project name is smoketcp

@Dieterbe

Copy link
Copy Markdown
Contributor

also, so this runs for you right? because i notice also the statsd object is now created differently

@jforest

jforest commented Oct 29, 2014

Copy link
Copy Markdown
Author

You're correct, I should remove the smokeping part, and will do so.

I modified it to use statsd.New instead of statsd.Dial, but I will change it back to minimize changes.

@jforest

jforest commented Oct 29, 2014

Copy link
Copy Markdown
Author

Ah, it's not dropping it in the right place, I didn't understand how the statsd module worked, let me modify it again, sorry.

…om all the other functions, output appears correct now
@jforest

jforest commented Oct 29, 2014

Copy link
Copy Markdown
Author

Ok, I believe it should now work as advertised, sorry for the back and forth.

@jforest

jforest commented Oct 30, 2014

Copy link
Copy Markdown
Author

This is functional, and I have another pull request that will stack on top of it, adding flags for the command line, and allowing you to set the interval time for the tests, would you like a second pull request or for me to merge them into this one?

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