----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30459/#review70468 -----------------------------------------------------------
Thanks for following up on the cleanup! Looks much better! Some higher level feedback per our chat: (1) Can we make this header more minimal by moving out everything except `validation::task::validate(...)` and `validation::offer::validate(...)`. There's a few reasons to motivate this, first is that this makes it easier to read this header and understand what the caller (master) should care about (right now it's a bit tough to come in to this header and "grok" it). Also, the other helpers still have a bit of dependencies between them (e.g. `validateResourceUsage` assumes the `ExecutorInfo` is valid) so we might want to avoid anyone calling these just yet :). If we want to test these, we can move them back to the header but hide them below the caller's API with a NOTE that they are exposed for testing :) (2) Right now this calls every validator even if the previous validators fail. That's different from the existing semantics and it's unnecessary work. Could we use lambda::bind per our chat to avoid this? - Ben Mahler On Jan. 30, 2015, 10:27 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30459/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2015, 10:27 p.m.) > > > Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone. > > > Bugs: MESOS-2305 > https://issues.apache.org/jira/browse/MESOS-2305 > > > Repository: mesos > > > Description > ------- > > Refactored task/offer/resource valiation in master. > > See ticket for motivation. > > > Diffs > ----- > > src/Makefile.am 07bea1fb8f0035413f2119859e16fa4f9383f68e > src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 > src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 > src/master/validation.hpp PRE-CREATION > src/master/validation.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30459/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
