----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35797/#review91955 -----------------------------------------------------------
src/master/master.hpp (line 534) <https://reviews.apache.org/r/35797/#comment145723> s/capabilities_size()/capabilities.size()/ src/master/master.hpp (line 536) <https://reviews.apache.org/r/35797/#comment145722> Do you also want to deal with the case where capabilities are previously set but now being unset? In other words, the "else" case. if (source.capabilities.size() > 0 ) { info.mutable_capabilities()->CopyFrom(source.capabilities); } else { info.clear_capabilities(); } If you want do this in a follow up patch, add a TODO here. You will also need a new test for this. src/tests/fault_tolerance_tests.cpp (line 1730) <https://reviews.apache.org/r/35797/#comment145724> Add a comment on the top of the test that you are verifying that when a framework re-registers with updated framework info, it gets updated in the master. src/tests/fault_tolerance_tests.cpp (lines 1742 - 1743) <https://reviews.apache.org/r/35797/#comment145720> We now require a much newer gcc version, so this shouldn't be an issue. You can just do. FrameworkInfo framework1 = DEFAULT_FRAMEWORK_INFO; src/tests/fault_tolerance_tests.cpp (lines 1769 - 1770) <https://reviews.apache.org/r/35797/#comment145725> ditto. just assign on the same line. src/tests/fault_tolerance_tests.cpp (line 1772) <https://reviews.apache.org/r/35797/#comment145726> s/Name/Type/ src/tests/fault_tolerance_tests.cpp (lines 1773 - 1774) <https://reviews.apache.org/r/35797/#comment145730> Can you also update webui_url, hostname and failover_timeout while you are at it? src/tests/fault_tolerance_tests.cpp (line 1803) <https://reviews.apache.org/r/35797/#comment145732> s/masterState/response/ src/tests/fault_tolerance_tests.cpp (line 1806) <https://reviews.apache.org/r/35797/#comment145733> s/masterStateObject/parse/ you also need a ASSERT_SOME(parse); src/tests/fault_tolerance_tests.cpp (line 1809) <https://reviews.apache.org/r/35797/#comment145734> new line after ASSERT. s/masterStateObject_/state/ src/tests/fault_tolerance_tests.cpp (line 1812) <https://reviews.apache.org/r/35797/#comment145736> s/famework_/framework/ new line after this. src/tests/fault_tolerance_tests.cpp (line 1831) <https://reviews.apache.org/r/35797/#comment145737> Nice test! - Vinod Kone On July 16, 2015, 6:46 p.m., Aditi Dixit wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35797/ > ----------------------------------------------------------- > > (Updated July 16, 2015, 6:46 p.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2880 > https://issues.apache.org/jira/browse/MESOS-2880 > > > Repository: mesos > > > Description > ------- > > This only updates the master, not the allocator. Added test too. > > > Diffs > ----- > > src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec > src/tests/fault_tolerance_tests.cpp > 1070ccf17f98f6b3800684a5edd6517d90631c3e > > Diff: https://reviews.apache.org/r/35797/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Aditi Dixit > >
