> On Jan. 6, 2015, 5:26 a.m., Niklas Nielsen wrote: > > Hi Kapil, > > > > One high-level observation is that hooks are wired up with function > > pointers. Can't we use an abstract class with virtual methods instead? > > Kapil Arya wrote: > It can be done either way. One of the reasons for using the functions > pointers was efficiency. Suppose we have a hundred available hook routines, > however, the hook module implements only 4 of them. And if there are 50 such > modules, we can easily imagine going through all 50 modules even though they > don't implement most hooks. > > Having said that, an abstract (that was our initial design) is cleaner > and I wouldn't mind doing that either. In that case, the return type for the > hooks can be changed to Try and the virtual function definition in the > abstract class will simply return Try<Nothing>. > > Until we decide on the high level design, I will hold off on replying to > other comments to avoid additional noise.
I would optimize for readability, avoiding to check for null function pointers and having a pattern that looks like the other intefaces. The Hook interface can provide default noop-implementations, so the subclassed hook implementations only need to implement what's necessary. Also, being able to deal with error cases is important. - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28655/#review66813 ----------------------------------------------------------- On Jan. 5, 2015, 2:42 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28655/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2015, 2:42 p.m.) > > > Review request for mesos and Niklas Nielsen. > > > Bugs: MESOS-2060 > https://issues.apache.org/jira/browse/MESOS-2060 > > > Repository: mesos-git > > > Description > ------- > > Hooks are implemented as Mesos modules. The first hook is a task > label-decorator that is called during the task launch sequence in Mesos > master. > > This hook allows hook modules to add additional labels to the incoming > TaskInfo object. The labels are then read on the slave/executor side which > may then act upon them. > > > Diffs > ----- > > src/Makefile.am 0521f5849acc3237a8fa3970c983beab74441586 > src/hook/hook.hpp PRE-CREATION > src/hook/manager.hpp PRE-CREATION > src/hook/manager.cpp PRE-CREATION > src/master/flags.hpp 1cea50c02f3ad7de1e1ae91d65d1accdb9af7b03 > src/master/main.cpp 193d53f13d8b14638b311cc290b5a5802ea56299 > src/master/master.cpp d6651e299ddb73bfdc1b126c474075db6cda8acd > src/module/hook.hpp PRE-CREATION > src/module/manager.cpp 5e779e4c565ee13206cee59e9097499320474187 > > Diff: https://reviews.apache.org/r/28655/diff/ > > > Testing > ------- > > Added two tests (see https://reviews.apache.org/r/29496/) and ran make check. > > > Thanks, > > Kapil Arya > >
