----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52347/#review151555 -----------------------------------------------------------
Some high level comments. Would take a detailed look again after another iteration. include/mesos/master/master.proto (lines 464 - 469) <https://reviews.apache.org/r/52347/#comment220001> Since the API is experimental, it should be fine to not worry about backward compatibility for now. Let's remove these fields except `framework_id` that is not present in `TaskStatus`. include/mesos/master/master.proto (line 470) <https://reviews.apache.org/r/52347/#comment219996> s/optional/required? include/mesos/master/master.proto (line 472) <https://reviews.apache.org/r/52347/#comment219997> 1. s/optional/required 2. s/Task/TaskStatus We don't need to send the entire `Task` to the user again. Just `TaskStatus` should suffice since it's just an update. Were you using any other field from the `Task` message? 3. Can you also add a comment that this `TaskStatus` update is for the last update acknowledged by the scheduler? src/master/master.cpp (line 7484) <https://reviews.apache.org/r/52347/#comment220004> nice cleanup! src/master/master.cpp (line 7536) <https://reviews.apache.org/r/52347/#comment220005> Can fit in one line? - Anand Mazumdar On Sept. 28, 2016, 1:48 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52347/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2016, 1:48 p.m.) > > > Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Vinod Kone. > > > Bugs: MESOS-5936 > https://issues.apache.org/jira/browse/MESOS-5936 > > > Repository: mesos > > > Description > ------- > > Send entire updated task in `TaskUpdated`. > > > Diffs > ----- > > include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db > include/mesos/v1/master/master.proto > 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 > src/common/protobuf_utils.hpp c5e5a9ad10134bcf87b59460bec7152e4916c75d > src/common/protobuf_utils.cpp 3ff37f498d6c830eb2f4b3d9395f9416cb01f5d3 > src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 > src/tests/api_tests.cpp e857b17cfe5f05d59859263c025564d33700a26c > > Diff: https://reviews.apache.org/r/52347/diff/ > > > Testing > ------- > > unit test. > > > Thanks, > > Zhitao Li > >
