mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 54650: Added validation for roles in ACCEPT call.
Date Tue, 13 Dec 2016 03:05:10 GMT


> On Dec. 12, 2016, 9:41 p.m., Qian Zhang wrote:
> > src/master/validation.cpp, line 1337
> > <https://reviews.apache.org/r/54650/diff/1/?file=1581654#file1581654line1337>
> >
> >     I would suggest to change to:
> >     ```
> >     return Error("Offer " + stringify(offerId) + " is no longer valid");
> >     ```
> 
> Jay Guo wrote:
>     We tend to put caller available info in their own error message instead of callee's,
see https://reviews.apache.org/r/54062/ for detailed explaination from BenM.
> 
> Benjamin Mahler wrote:
>     Jay: that would be the general rule we follow currently, but I'm not sure we follow
this during validation (due to the way the calling code is written). We should make sure the
final validation error message that we'll log or return to the framework includes the offer
id. Does it?
> 
> Jay Guo wrote:
>     IINM, OfferId is printed here: https://github.com/apache/mesos/blob/master/src/master/master.cpp#L3530

That is just the warning message we log in master side, what about the error message that
we will return to framework? I do not think OfferId is included currently: https://github.com/apache/mesos/blob/master/src/master/master.cpp#L3562


- Qian


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


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