mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jay Guo <guojiannan1...@gmail.com>
Subject Re: Review Request 54650: Added validation for roles in ACCEPT call.
Date Tue, 13 Dec 2016 02:28:43 GMT


> On Dec. 13, 2016, 8:40 a.m., Benjamin Mahler wrote:
> > src/master/validation.cpp, line 1425
> > <https://reviews.apache.org/r/54650/diff/2/?file=1581993#file1581993line1425>
> >
> >     This return here looks incorrect? Maybe copy over a similar comment from validateSlave:
> >     
> >     ```
> >         // Use the first role to validate against the rest.
> >         if (role.isNone()) {
> >           role = _role.get();
> >         }
> >     ```

hmm... why do we want to continue following validation if we hit this base case? Next check
would pass would be the same anyway since we have just set them to be identical. I think we
could return `None()` for `validateSlave` as well...


- Jay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54650/#review158937
-----------------------------------------------------------


On Dec. 12, 2016, 10:30 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54650/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2016, 10:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6637
>     https://issues.apache.org/jira/browse/MESOS-6637
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> For a multi-role framework, it may receive offers for different roles
> in that framework. However, we disallow multiple offers with different
> roles being accepted in single ACCEPT call.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 96aa36585ded4bd7cf98526f710ccbc4f23b1f0f 
> 
> Diff: https://reviews.apache.org/r/54650/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message