----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72955/#review222039 -----------------------------------------------------------
src/master/master.hpp Lines 1061-1062 (original), 1061-1062 (patched) <https://reviews.apache.org/r/72955/#comment311107> Maybe just say "based on master flags and the framework not being completed"? Also, this makes it sound like it only validates things against the master flags and the framework being non-completed, but it also validates somewhat that the FrameworkInfo is valid on its own, like the failover timeout. Should the failover timeout validation be moved to the existing stateless validation of FrameworkInfo? src/master/master.cpp Lines 2588-2598 (original) <https://reviews.apache.org/r/72955/#comment311108> Per above comment, did you mean to also move out the failover timeout validation, which looks to be a similarly stateless validation of the FrameworkInfo as this bit? src/master/master.cpp Lines 2629-2634 (patched) <https://reviews.apache.org/r/72955/#comment311109> Hm.. shouldn't this stateless FrameworkInfo validation go in the existing `validation::framework::valdiate(const FrameworkInfo&)` function in master/validation.hpp? - Benjamin Mahler On Oct. 12, 2020, 8:12 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72955/ > ----------------------------------------------------------- > > (Updated Oct. 12, 2020, 8:12 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10176 > https://issues.apache.org/jira/browse/MESOS-10176 > > > Repository: mesos > > > Description > ------- > > This merges three near-identical pieces of scattered code in SUBSCRIBE > and UPDATE_FRAMEWORK execution paths in the Master that validate > and construct parts of `allocator::FrameworkOptions` (the set of > suppressed roles and the offer constraints filter) into a single > function. > > This is a prerequisite to adding validation of offer constraint roles. > > > Diffs > ----- > > src/master/master.hpp 83d8190702ff25b5e3e9607d8250aa331aaad028 > src/master/master.cpp d6d3ea7e27539a65b601783eca2ac4e3b688f188 > > > Diff: https://reviews.apache.org/r/72955/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Andrei Sekretenko > >
