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 Fri, 04 Nov 2016 21:13:25 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.

+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 '" +

...
```


- 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