----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52470/#review151851 -----------------------------------------------------------
can you update tests to ensure we dont set executorid in http endpoints for command tasks? src/common/protobuf_utils.cpp (lines 217 - 221) <https://reviews.apache.org/r/52470/#comment220466> the `else if` is a bit confusing. i would rather just have the `if executorId.isSome()` and make sure the callers correctly call createTask() with `executorId` set if it's not a command executor. to make sure we catch all the call sites, i would also not have a default value (of `None`) for the `executorId` argument. src/slave/http.cpp (line 1409) <https://reviews.apache.org/r/52470/#comment220453> so you are setting executor id here for command tasks as well!? i don't know if that's backwards compatible. you should probably only set it if it's not a command executor. src/slave/slave.cpp (line 1403) <https://reviews.apache.org/r/52470/#comment220454> ditto. src/slave/slave.cpp (line 6511) <https://reviews.apache.org/r/52470/#comment220455> ditto. src/slave/slave.cpp (line 6558) <https://reviews.apache.org/r/52470/#comment220456> ditto. src/slave/slave.cpp (line 6645) <https://reviews.apache.org/r/52470/#comment220459> ditto. src/slave/slave.cpp (line 6670) <https://reviews.apache.org/r/52470/#comment220457> ditto. - Vinod Kone On Oct. 7, 2016, 6:09 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52470/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2016, 6:09 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-6283 > https://issues.apache.org/jira/browse/MESOS-6283 > > > Repository: mesos > > > Description > ------- > > Set executor id in `protobuf::createTask`. > > > Diffs > ----- > > src/common/protobuf_utils.hpp 96df060d48d0206cfd745e17dcd568d0fbd032f4 > src/common/protobuf_utils.cpp f09b0692ad5b47ac3f4c755a3921d15c242778b4 > src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 > src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 > src/slave/http.cpp 79061c3cd94d856ec695e5a82bf6792bf089d1f8 > src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 > > Diff: https://reviews.apache.org/r/52470/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
