----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36036/#review89933 -----------------------------------------------------------
src/common/protobuf_utils.cpp (line 145) <https://reviews.apache.org/r/36036/#comment142825> We should've added a comment on this method that this is only used by the StandaloneMasterDetector (used in tests and outside tests when ZK is not used). For example when we start a slave with [email protected]:5051, since the slave (and consequently its detector) doesn't have enough information about MasterInfo, it tries to construct it based on the available information (UPID). Mind adding this comment? So, I don't think it makes sense for us to add the version here, because the slave/framework (detector) doesn't know what version the master is running. What you are doing is setting the version of slave (or libmesos in case of framework) in MasterInfo! src/common/protobuf_utils.cpp (line 162) <https://reviews.apache.org/r/36036/#comment142826> Kill this. See above. Do this in Master::initialize() where the MasterInfo is created by the master. src/tests/master_contender_detector_tests.cpp (lines 185 - 186) <https://reviews.apache.org/r/36036/#comment142828> Per my comment above, a test using StandaloneMasterDetector shouldn't be testing version. So kill this block. I would recommend adding this check in a test that uses ZooKeeperMasterDetector as thats what production clusters use. See MasterZookeerTest in master_tests.cpp for inspiration. - Vinod Kone On June 30, 2015, 6:28 a.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36036/ > ----------------------------------------------------------- > > (Updated June 30, 2015, 6:28 a.m.) > > > Review request for mesos, Anand Mazumdar, Isabel Jimenez, and Vinod Kone. > > > Bugs: MESOS-2957 > https://issues.apache.org/jira/browse/MESOS-2957 > > > Repository: mesos > > > Description > ------- > > Jira: MESOS-2957 > > Adds an optional version string to the `MasterInfo` protobuf, > to simplify handling versioning in HTTP API calls. > > The version string is taken from <mesos/version.hpp> which is the > same used when starting up Master (master/main.cpp). > > I've added a simple test on the back of the `MasterDetector` test, > as this was the place where we would expect the `MasterInfo` to > have been fully populated by real production code (as opposed to > other places, where it is actually handled by mocks). > > > Diffs > ----- > > include/mesos/mesos.proto 0ebe5d3a5ae033419482c28be80b09d277e0ff02 > src/common/protobuf_utils.cpp f0642ba12b914889a876d1d1c49a2c242006433c > src/common/type_utils.cpp f7b21c6abc392fec0514224ff02a00e772389f5d > src/tests/master_contender_detector_tests.cpp > d7a3b46b2e437818631064ae34317e49c9aa3748 > > Diff: https://reviews.apache.org/r/36036/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Marco Massenzio > >
