mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.
Date Sun, 06 Nov 2016 18:52:12 GMT


> On Nov. 4, 2016, 7: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 '" +
>     
>     ...
>     ```

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.


- Neil


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


On Nov. 1, 2016, 4:54 p.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52587/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2016, 4:54 p.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