> On Jan. 21, 2020, 2:40 p.m., Andrei Sekretenko wrote: > > src/slave/slave.cpp > > Lines 10625 (patched) > > <https://reviews.apache.org/r/72035/diff/1/?file=2209256#file2209256line10625> > > > > Can you avoid spliting initialization of `isGeneratedForCommandTask_` > > and just fully initialize it here? > > After that, it should also be possible to make it `const`. > > > > Btw, it might make sense to move the workaround logic into > > `ExecutorState::recover(...)` so that we don't have Option in the code.
Fixed the initialization. Still defined the member as mutable as nothing about it makes it inherently unassignable; if one worries about unintentionally mutating internal state one should instead either work with `const` `Executor` variables in the calling scope, or declare `Executor` member functions as `const`. We don't do the first often enough, but often enough adhere to the second. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72035/#review219342 ----------------------------------------------------------- On Jan. 22, 2020, 1:46 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72035/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2020, 1:46 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-10084 > https://issues.apache.org/jira/browse/MESOS-10084 > > > Repository: mesos > > > Description > ------- > > This patch adds code to pass on whether was generated in the agent from > the point where the executor is generated to the point where we create > an actual `slave::Executor` instance. This allows us to persist this > information in the executor state. > > > Diffs > ----- > > src/slave/slave.hpp 3d191dcbdd58e851ad5e86fbb01a890407409c64 > src/slave/slave.cpp 0005971717f5b90da368b45caad8e209ada95fa5 > src/tests/mock_slave.hpp eb31a0f93d21ef859684c26a99effde08348ead6 > src/tests/mock_slave.cpp 4940285b183a9513701e616e7d7523801daf113b > > > Diff: https://reviews.apache.org/r/72035/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
