> On 十月 13, 2015, 2:52 a.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? > > Niklas Nielsen wrote: > 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?
Hi Niklas, my orignal thinking is that we can enhance the API to HookManager::hooksAvailable(const string& hook), but the slave do not know each hook's real function, so we cannot check this in slave. Yes, your explanation does makse sense, thanks! ;-) - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38564/#review102380 ----------------------------------------------------------- On 十月 13, 2015, 1:39 a.m., Felix Abecassis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38564/ > ----------------------------------------------------------- > > (Updated 十月 13, 2015, 1:39 a.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 > >
