> On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: > > src/slave/slave.cpp, line 1111 > > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line1111> > > > > Can we instead swap its name with `task_` and move the label decorator > > higher (right after entry into this function)? > > > > This would prevent future developers from accidentally using `task` > > when they meant to use `task_`. > > > > Doing the label decoration before the error checks has an added benefit > > of generating notifications for dropped tasks as well for the hook modules.
I don't think we should reveal task infos that are failing. How should the module implementer know the difference between a successfully launched task and a failed one? The pattern is to append '_' as f(), f'(), f''(). So would keep that for consistency. > On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: > > src/slave/slave.cpp, line 3926 > > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line3926> > > > > const ref? Great catch! Thanks > On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote: > > src/slave/slave.cpp, lines 3924-3934 > > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line3924> > > > > Should we rather move it close to the runTask definition? Since the two > > are logically close. The ordering is slightly inconsistent (should follow the one from the header file). I'd prefer not to cludder the code and methods around runTask() with helpers - there is a lot of central pieces there already. Are you feeling strongly about moving this up? - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31016/#review73457 ----------------------------------------------------------- On March 13, 2015, 4:04 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31016/ > ----------------------------------------------------------- > > (Updated March 13, 2015, 4:04 p.m.) > > > Review request for mesos, Ben Mahler and Kapil Arya. > > > Repository: mesos > > > Description > ------- > > Added decorator which gets invoked on start of runTask() sequence in the > slave. > > > Diffs > ----- > > include/mesos/hook.hpp 9ae8b9455a86c7a5cbf4f1d1b1ce88f2811ce35d > src/hook/manager.hpp da813492108974a7e26b366845368517589da876 > src/hook/manager.cpp 7a4cb09bc221af502e867cfb7fff2900b599ff1f > src/slave/slave.hpp 989832f8783d07d0702b30f0a68b8c383b57c621 > src/slave/slave.cpp 0f99e4efb8fa2b96f120a3e49191158ca0364c06 > > Diff: https://reviews.apache.org/r/31016/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
