----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review161488 -----------------------------------------------------------
src/master/master.cpp (lines 3944 - 3950) <https://reviews.apache.org/r/55461/#comment232788> I think we explicitly disallow empyt role field? https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L67 Also, should the case `framework->info.has_role() == false && framework->info.role() != "*"` be invalid and caught upon subscription? - Jay Guo On Jan. 12, 2017, 10:54 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55461/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2017, 10:54 p.m.) > > > Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu. > > > Repository: mesos > > > Description > ------- > > This updates the resource reservation validation for frameworks which > can have multiple roles. During a deprecation period 'FrameworkInfo' > will have fields for both 'role' and 'roles', however the validation > function works with just an optional set of roles. Here an empty set > captures the previous semantics of either having an empty 'role' field > or 'role' set as '*'. This forces the callers to properly construct a > set of framework roles from the available information. An optional set > is used in order to accommodate callers which have no information > about the framework's roles, and ultimately disables validation taking > that information into account. > > > Diffs > ----- > > src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040 > src/master/validation.hpp 57e81779ff7444904c2ad7bad33aaf9167b98d05 > src/master/validation.cpp 96aa36585ded4bd7cf98526f710ccbc4f23b1f0f > src/tests/master_validation_tests.cpp > e5d55e03648cb218d42adc594d6fa7d40ea9bcbb > > Diff: https://reviews.apache.org/r/55461/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Bannier > >