mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.
Date Mon, 07 Nov 2016 22:16:17 GMT


> On Nov. 4, 2016, 12:30 p.m., Neil Conway wrote:
> > src/master/validation.cpp, line 1532
> > <https://reviews.apache.org/r/52587/diff/3/?file=1550774#file1550774line1532>
> >
> >     Can we use consistent tense here? The other volume error messages say "has been
attempted" rather than "being attempted".
> >     
> >     Also, the other error messages don't include the volume that failed validation,
so we should probably be consistent.
> 
> Jiang Yan Xu wrote:
>     +1 on the tense.
>     
>     but re: "the other error messages don't include the volume that failed validation"
they don't include the volume but they include other things this error message doesn't include.
Whether the additional information is useful depends on the failure IMO. Nevertheless I guess
it's true we don't have to say it's a shared volume separately because `stringify(volume)`
reveals it.
>     
>     
>     So overall the following look good?
>     
>     ```
>     ...
>     
>     "Create volume operation for '" + stringify(volume) +
>     "' has been attempted by framework '" +
>     
>     ...
>     ```
> 
> Neil Conway wrote:
>     Why do you think this particular error message warrants including `stringify(volume)`
any more/less than the other places where a volume fails validation? 
>     
>     I'm not opposed to including more context in validation failure messages, but I think
we should also be consistent.

My critiera for adding `stringify(volume)` is that it will print the sharedness of the volume
so it's obvious in the log w.r.t the operation: which volume is problematic, what makes it
problematic, why it is problematic.

However examining the two other error messages closely, I think they should be given the same
treatment. I'll send a RR.


- Jiang Yan


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


On Nov. 1, 2016, 9:54 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52587/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2016, 9:54 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6316
>     https://issues.apache.org/jira/browse/MESOS-6316
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework issues a CREATE for a shared volume, we allow that
> operation only if the framework has opted in for the capability of
> SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
> do not check as the operator API is not related to a specific
> framework.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 013bb592ba47b785c552e199633e4784e8aa71b1 
>   src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
>   src/master/validation.cpp f690a9eacd278b51a52f5588dbeea377df074435 
>   src/tests/master_validation_tests.cpp a5d8610bd61822cdf55cbc8d7056e5cf8fecfa54 
>   src/tests/persistent_volume_tests.cpp 6289009fe9ed0a57ba5eff46dbbe0633a75d2616 
> 
> Diff: https://reviews.apache.org/r/52587/diff/
> 
> 
> Testing
> -------
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


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