----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46053/#review128277 -----------------------------------------------------------
Patch looks great! Reviews applied: [45969, 46053] Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh - Mesos ReviewBot On April 11, 2016, 10:11 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46053/ > ----------------------------------------------------------- > > (Updated April 11, 2016, 10:11 p.m.) > > > Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya. > > > Bugs: MESOS-5178 > https://issues.apache.org/jira/browse/MESOS-5178 > > > Repository: mesos > > > Description > ------- > > We considered adding this logic directly to the > 'Resources::validate()' function, but ultimately decided against it. > The primary reason is that the existing 'Resources::validate()' > function doesn't consider the semantics of any particular resource > when performing its validation (it only makes sure that the fields in > the 'Resource' protobuf message are correctly formed). Since a > fractional 'gpus' resources is actually well-formed (and only > semantically incorrect), we decided to push this validation logic up > into the master. > > Moreover, the existing logic to construct a 'Resources' object from a > 'RepeatedPtrField<Resource>' silently drops any resources that don't > pass 'Resources::validate()'. This means that if we had pushed the > non-fractional 'gpus' validation into 'Resources::validate()', the > 'gpus' resources would just have been silently dropped rather than > causing a TASK_ERROR in the master. This is obviously *not* the > desired behaviour. > > > Diffs > ----- > > src/master/validation.cpp 504cd9b8bd5d40bb591b7aa5a23bd74cc210c2fc > > Diff: https://reviews.apache.org/r/46053/diff/ > > > Testing > ------- > > Test coming in subsequent commit. > > > Thanks, > > Kevin Klues > >
