> On March 16, 2015, 3:40 p.m., Adam B wrote: > > src/slave/slave.cpp, lines 2490-2491 > > <https://reviews.apache.org/r/32130/diff/1/?file=896443#file896443line2490> > > > > Why not just use update.mutable_status() instead of making a copy of > > the StatusUpdate object? We do this in several other places: > > ``` > > TaskStatus* status = update.mutable_status(); > > status->set_source(...); > > ``` > > e.g. src/sched/sched.cpp > > Then we can get rid of all the update_'s > > Niklas Nielsen wrote: > You don't have mutable_status on a const update object :) We would need > to unconst the argument then and wouldn't need to copy in the first place.
Fair enough. LGTM! - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32130/#review76666 ----------------------------------------------------------- On March 16, 2015, 4:06 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32130/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 4:06 p.m.) > > > Review request for mesos, Niklas Nielsen and Vinod Kone. > > > Bugs: MESOS-2499 > https://issues.apache.org/jira/browse/MESOS-2499 > > > Repository: mesos > > > Description > ------- > > A status update originating from executor should have the TaskStatus::source > field set to TaskStatus::SOURCE_EXECUTOR. Set this field in the slave to be > future proof (a future where there will be no executor driver). Previous code > has a bug and updated a copy of the update that was not forwarded. Add some > checks for source and reason for status updates in existing tests. > > > Diffs > ----- > > src/slave/slave.cpp 0f99e4efb8fa2b96f120a3e49191158ca0364c06 > src/tests/slave_tests.cpp a975305430097a8295b4b155e8448572c12bde22 > > Diff: https://reviews.apache.org/r/32130/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.5, CentOS 7.0.1406) > > > Thanks, > > Alexander Rukletsov > >
