> On Nov. 16, 2018, 1:01 p.m., Benjamin Bannier wrote: > > src/master/validation.cpp > > Lines 2531-2535 (patched) > > <https://reviews.apache.org/r/69356/diff/1/?file=2108397#file2108397line2531> > > > > I wonder whether performing this validation in the master is the right > > thing to do. > > > > Profiles are currently applied by SLRP in the agent, so doing > > SLRP-specific validation here not only scatters logic in different places, > > but could also make making breaking changes in the future harder than > > necessary (agent and master are upgraded independently). > > > > Should we move this into the agent, e.g., SLRP? > > Chun-Hung Hsiao wrote: > It seems to me that if this is the semantics in the API, then it seems to > me that we should validate this as early as possible to fail-fast. WDYT? > > Benjamin Bannier wrote: > I agree with failing as early as possible. This should not happen at the > cost of making master even more of an God object than it already is, though. > Does that make sense? > > James DeFelice wrote: > AFAICT there's already a bunch of validation in the master, specifically > for this API. If we want to move this validation out of the master and into > SLRP, shouldn't that be tackled separately? It sounds like a bigger > (philosophical) change than simply adding an additional validation rule to a > set of rules that already exist on the master.
Chun, could you create a follow-up ticket to possible move validation beyond resource and proto validity to SLRP? Dropping for now. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69356/#review210601 ----------------------------------------------------------- On Nov. 19, 2018, 10:24 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69356/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2018, 10:24 p.m.) > > > Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu. > > > Bugs: MESOS-9275 > https://issues.apache.org/jira/browse/MESOS-9275 > > > Repository: mesos > > > Description > ------- > > Added validation for `Offer.Operation.CreateDisk.target_profile`. > > > Diffs > ----- > > src/master/validation.cpp 5768ac8fe802f28855fbd7be135c711115532771 > src/tests/master_validation_tests.cpp > aa7c8f70c09459be32c6c415497e95fcdc218efd > > > Diff: https://reviews.apache.org/r/69356/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
