> On Oct. 12, 2015, 7:52 p.m., Guangya Liu wrote: > > src/slave/slave.cpp, line 394 > > <https://reviews.apache.org/r/38564/diff/4/?file=1096923#file1096923line394> > > > > I have one question for this after second review: I see that in > > slave.cpp, most APIs are calling HookManager::hooksAvailable() to check if > > there are hook managers. The problem is that if I have two hooks hookA and > > hookB, then the HookManager::hooksAvailable() will return True but the call > > in this code block is calling hookC which will be failed. > > > > Do we need to enhance HookManager::hooksAvailable() by adding a > > parameter to check the availability for a specified hook manager?
It is a best-effort short circuit: we know for sure that there won't be anything to call if no hooks have been supplied. The hookC in your example will have a default implementation and be a noop if not supplied by the user. Does this make sense, or did you think of something else? If so, would you mind if we drop the issue here? - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38564/#review102380 ----------------------------------------------------------- On Oct. 12, 2015, 6:39 p.m., Felix Abecassis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38564/ > ----------------------------------------------------------- > > (Updated Oct. 12, 2015, 6:39 p.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > ------- > > Add a new callback enabling custom attribute discovery logic > > > Diffs > ----- > > include/mesos/hook.hpp 0c1042a > src/examples/test_hook_module.cpp cd7c184 > src/hook/manager.hpp 3af1ff8 > src/hook/manager.cpp 108bd46 > src/slave/slave.cpp 01c5e42 > src/tests/hook_tests.cpp b35ce72 > > Diff: https://reviews.apache.org/r/38564/diff/ > > > Testing > ------- > > > Thanks, > > Felix Abecassis > >
