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




src/master/master.cpp
Lines 4949 (patched)
<https://reviews.apache.org/r/66050/#comment282002>

    Nit:
    s/in operator API/in the operator API/
    
    Here and elsewhere.



src/master/master.cpp
Lines 4986-4994 (patched)
<https://reviews.apache.org/r/66050/#comment282048>

    What will happen if these operations are sent to a 1.5 agent which has the 
RESOURCE_PROVIDER capability, but does not support the operation?



src/master/master.cpp
Lines 5029 (patched)
<https://reviews.apache.org/r/66050/#comment282015>

    Suggestion:
    s/with subtract/subtracting/



src/master/master.cpp
Lines 5331-5334 (original), 5467-5472 (patched)
<https://reviews.apache.org/r/66050/#comment282018>

    This change doesn't really belong in this patch. Also, if we're going to 
update this location for consistency, then we should update other similar 
occurrences in this function as well (like DESTROY_VOLUME below).
    
    I'd recommend either following up with a separate cleanup patch, or leaving 
this as-is.



src/master/validation.cpp
Lines 2344 (patched)
<https://reviews.apache.org/r/66050/#comment282039>

    Are you sure this will definitely result in a scalar value of zero? I think 
protobuf will default-initialize the double in the `value` field, which leaves 
it in an indeterminate state?
    
    I think we should do `zero.set_value(0);` before the comparison is 
performed.
    
    Ditto for the shrink_volume validator below.



src/master/validation.cpp
Lines 2346 (patched)
<https://reviews.apache.org/r/66050/#comment282046>

    Suggestion:
    "The size of the 'addition' volume in 'grow_volume' must be greater than 
zero"
    
    If you end up enclosing 'grow_volume' in quotes here, then I would do it in 
other error messages in these functions as well.



src/master/validation.cpp
Lines 2355 (patched)
<https://reviews.apache.org/r/66050/#comment282040>

    s/Not a/Invalid/
    
    Here and below.



src/master/validation.cpp
Lines 2359 (patched)
<https://reviews.apache.org/r/66050/#comment282041>

    Nit:
    s/Growing/Growing a/



src/master/validation.cpp
Lines 2392 (patched)
<https://reviews.apache.org/r/66050/#comment282042>

    s/cannot be zero/must be greater than zero/



src/master/validation.cpp
Lines 2397 (patched)
<https://reviews.apache.org/r/66050/#comment282047>

    Suggestion:
    "The value of 'subtract' in 'shrink_volume' must be smaller than the 
persistent volume's size"
    
    If you end up enclosing 'shrink_volume' in quotes here, then I would do it 
in other error messages as well.



src/master/validation.cpp
Lines 2412 (patched)
<https://reviews.apache.org/r/66050/#comment282044>

    Nit:
    s/Shrinking/Shrinking a/


- Greg Mann


On April 10, 2018, 4:18 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> -----------------------------------------------------------
> 
> (Updated April 10, 2018, 4:18 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The new offer operations are implemented as speculative operations, but
> we will use validation to make them non-speculative on API level so that
> we can transition later without a breaking change.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp f7da675e8fe96159e5335c9e83b65b67ed90eda8 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Reply via email to