-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review100296
-----------------------------------------------------------

Ship it!



src/slave/slave.cpp (line 375)
<https://reviews.apache.org/r/38473/#comment157469>

    Shouldn't this be doing what the master is doing as well and just setting 
the hostname to the stringified IP?



src/tests/cluster.hpp (line 134)
<https://reviews.apache.org/r/38473/#comment157460>

    Why is the variable called 'upid' instead of just 'pid'? This is not 
actually a `UPID`. Also, why not just call this 'find' like you did for the 
function below? It seems weird to ever need to do `masters.findMaster(...)`, 
you'd just want `master.find(...)`.



src/tests/master_tests.cpp (lines 1091 - 1093)
<https://reviews.apache.org/r/38473/#comment157459>

    Unnecessary code.



src/tests/master_tests.cpp (line 1102)
<https://reviews.apache.org/r/38473/#comment157461>

    s/upid/pid/g



src/tests/master_tests.cpp (lines 1108 - 1109)
<https://reviews.apache.org/r/38473/#comment157464>

    Aren't you checking 'master.get()->info().hostname()' and expecting 
'pid.get().address.hostname()'? In otherwords, aren't these in the wrong 
ordering?



src/tests/master_tests.cpp (line 1124)
<https://reviews.apache.org/r/38473/#comment157463>

    Newline here like you did in test above please.



src/tests/master_tests.cpp (line 1127)
<https://reviews.apache.org/r/38473/#comment157465>

    I'm not sure what this tests for and if it doesn't actually introduce a 
potential source of flakiness? If we keep this, it needs a comment for why we 
are doing it.



src/tests/master_tests.cpp (line 1137)
<https://reviews.apache.org/r/38473/#comment157466>

    This shouldn't be here right?



src/tests/master_tests.cpp (line 1143)
<https://reviews.apache.org/r/38473/#comment157467>

    This test shouldn't fail should it? I think this entire test is no longer 
valid.


- Benjamin Hindman


On Sept. 22, 2015, 12:22 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Reply via email to