> On May 5, 2016, 1:27 p.m., Kapil Arya wrote: > > The hooks looks good but I have a high-level question. Instead of having > > two separate hooks for validation and label-decoration, I am wondering, if > > we can have one consolidated hook. That hook would then update the labels > > (and any other fields that we might want to in future) and also validate a > > task at the same time. > > > > Any thoughts? > > Joseph Wu wrote: > That might work, but I'm worried that this would make the semantics of > the hook somewhat odd. > > Most of the hooks have the `Result` return type, where `None` implies no > change, and `Error` implies the *hook errored*. This new hook introduces a > hook where an `Error` implies the *task errored*. To combine the two hooks > an maintain semantics, we'd have a return type of `tuple<Result<Labels>, > Try<Nothing>>` or something... > > Not to mention combining the hooks would be a breaking module interface > change...
I agree about the breaking changes. However, we might consider creating a protobuf, say SlaveTaskHookInfo, which would contain several optional fields including labels, Error, etc. In anycase, let's not worry about it for now. In future, if the need arises, we can reconsider the alternate. - Kapil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46920/#review131869 ----------------------------------------------------------- On May 5, 2016, 12:01 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46920/ > ----------------------------------------------------------- > > (Updated May 5, 2016, 12:01 p.m.) > > > Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya. > > > Repository: mesos > > > Description > ------- > > Adds a new hook `slaveRunTaskValidatorHook`, which allows a module to > inspect the contents of a task and potentially reject the task with a > `TASK_ERROR`. > > > Diffs > ----- > > include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca > src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 > src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 > src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 > src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 > src/tests/hook_tests.cpp 60d52c5849ba555f6f3070883d87aadf105f05b0 > > Diff: https://reviews.apache.org/r/46920/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Joseph Wu > >
