----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72898/#review221950 -----------------------------------------------------------
Ship it! Thanks for adding this, I think we should maybe just state what is valid, rather than assuming what the user's intent is for the invalid cases and providing suggestions of what to do instead? include/mesos/scheduler/scheduler.proto Lines 447-449 (patched) <https://reviews.apache.org/r/72898/#comment311017> Seems a bit odd to jump from one empty Group -> to all of `groups` to be empty? The user could just have 1 empty Group and other non empty Groups, in which case this suggestion seems not relevant? Perhaps we just say that Group and RoleConstraints if specified must be non-empty? include/mesos/scheduler/scheduler.proto Lines 449-451 (patched) <https://reviews.apache.org/r/72898/#comment311016> I think we can just say empty role constraints is not valid, rather than also talking about suppression here. We're assuming the behavior of empty RoleConstraints would be no offers, but one might interpret "no constraints" to me "any offer is ok", and seeing suppression mentioned here seems a bit confusing. If they know that it's not valid, they don't need to consider what its behavior is? - Benjamin Mahler On Sept. 24, 2020, 3:13 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72898/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2020, 3:13 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Added `OfferConstraints` validity criteria into protobuf comments. > > > Diffs > ----- > > include/mesos/scheduler/scheduler.proto > f70738ce5f4aeddb38c402130f953dcff1d43d6a > > > Diff: https://reviews.apache.org/r/72898/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
