> On Jan. 13, 2017, 4:15 p.m., Jay Guo wrote: > > src/master/master.cpp, lines 3944-3950 > > <https://reviews.apache.org/r/55461/diff/1/?file=1603853#file1603853line3944> > > > > 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? > > Benjamin Bannier wrote: > Multi-role frameworks would use the `roles` field, in which case the > `role` field would not be set. We need to distinguish here whether a > framework uses `role` or `roles`. The check can be simplified to > `framework->info.role() != "*"` though, independently of what we assume about > this field upstream. I made that change. > > Does that address your comment?
then why not using framework capability here? note that https://reviews.apache.org/r/55021/ is landed which you may want to use. - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review161488 ----------------------------------------------------------- On Jan. 13, 2017, 11:50 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55461/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2017, 11:50 p.m.) > > > Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu. > > > Bugs: MESOS-6730 > https://issues.apache.org/jira/browse/MESOS-6730 > > > 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 > >
