Slave advertise

[ch3lo]

This "PR" override the auto generated Slave Hostname.

You can assign the slave hostname with advertise parameter.

Signed-off-by: Marcelo Salazar chelosalazar@gmail.com

[mforsyth]

Hi @ch3lo, thanks for making this pull request!

You are assuming that the hostname that will be used to tag events is the same hostname that you want to use when connecting to the Mesos monitoring API. Although it wouldn't necessarily always be true, this would usually be a reasonable assumption (and it's definitely better than just hardcoding both to be lookups of the hostname of the local host) so :+1: .

However, let's still call the function "get-slave-host" (moving it to util as you have done is fine), and let's call the setting "slave-host". This value influences more than just "advertising" - it's also the identifier by which the mesos slave is reached. In both contexts "slave-host" describes the purpose of the value we're talking about.

Thanks again @ch3lo

[ch3lo]

Hi @mforsyth ,

I made the suggested changes. Is a pleasure collaborate with your project.

Regards

[ch3lo]

I rebase my branch with the last commit with --force.

I think all should be ok.

[mforsyth]

@c3hlo would you mind squashing these 2 commits down into 1? It will be better for the history since the second commit is just an adjustment of the first commit. After that we should be good to go! Thanks!

[ch3lo]

ready

[mforsyth]

Thanks again. I hate to be a pain @ch3lo but I just realized that in order for this to be merged you have to sign Two Sigma's CLA - it's mentioned in the Readme. Can you fill out that form & send it in as per instructions?

[ch3lo]

Sent

[mforsyth]

Thanks @ch3lo !