----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28618/#review63724 -----------------------------------------------------------
Ship it! src/master/master.cpp <https://reviews.apache.org/r/28618/#comment106011> Would love to see a separate change to do s/Checker/Validator/ and s/Visitor/Validator/ as a trivial change to make the code a bit easier to read and understand. Since this doesn't actually use the visitor pattern. Will be an easy change to commit, compared to the actual validator cleanup we need to do later. Feel free to punt. src/master/master.cpp <https://reviews.apache.org/r/28618/#comment106013> This check misses the case where a task explicitly uses no resources: cpus=0, for example. How about moving this line: ``` Resources resources = task.resources(); ``` Up to here, and then we could check empty correctly: ``` Resources taskResources = task.resources(); if (resources.empty()) { return Error("Task uses no resources"); } ``` - Ben Mahler On Dec. 3, 2014, 12:51 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28618/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2014, 12:51 a.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: mesos-2031 > https://issues.apache.org/jira/browse/mesos-2031 > > > Repository: mesos-git > > > Description > ------- > > Splitted resource and resource usage checkers. > > > Diffs > ----- > > src/master/master.cpp 8fcda4b9b5857f14cff8f6af2de31cca0208b88d > > Diff: https://reviews.apache.org/r/28618/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
