> On Nov. 7, 2015, 12:19 a.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 140 > > <https://reviews.apache.org/r/39285/diff/7/?file=1118136#file1118136line140> > > > > Suggest to move it into the loop; if any role is not known by master, > > we did not need to continue to check others. > > Alexander Rukletsov wrote: > I think the flow is more readable how it's now: in the loop we > reconstruct the "reference" role, afterwards we check whether it is known to > the master. Also, my gut feeling is that typos in roles will not be that > frequent, so checking it once instead of per resource makes sense to me. > > Joerg Schad wrote: > There should just be a single role per request, why should I check that > in the loop?
`::protobuf::parse()` will be heavy, we can end loop early if the role is not known by master. But as @AlexR said, typos in role is rare case, it also make sense to me to keep current behavior. > On Nov. 7, 2015, 12:19 a.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 180 > > <https://reviews.apache.org/r/39285/diff/7/?file=1118136#file1118136line180> > > > > Should we move it into `validateQuotaRequest`? If any role is exist in > > master, we did not need to continue to check others. And in QuotaHandler, > > we had the pointer to master `Master* master`. > > Alexander Rukletsov wrote: > I'd keep it here, because it's related to how we currently process the > request, rather than whether the request is valid. > > Joerg Schad wrote: > There was an earlier comment from Joris mentioning that this isn't really > part of validatating the request (as it also involves state of the master). Agree :). - Klaus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105440 ----------------------------------------------------------- On Nov. 9, 2015, 8:51 p.m., Joerg Schad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39285/ > ----------------------------------------------------------- > > (Updated Nov. 9, 2015, 8:51 p.m.) > > > Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van > Remoortere. > > > Bugs: MESOS-3199 > https://issues.apache.org/jira/browse/MESOS-3199 > > > Repository: mesos > > > Description > ------- > > Added Quota Request Validation. > > > Diffs > ----- > > src/master/master.hpp b76d30197b3decda0a742e03ce01a17a64b633ac > src/master/quota_handler.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/39285/diff/ > > > Testing > ------- > > make test > > > Thanks, > > Joerg Schad > >
