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


Nice, the cherry on top would be validating any user input:

mem <= system memory
disk <= system disk size
cpu <= system cpus


src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29301>

    Seems cleaner to not store the option:
    
    double cpus;
    if (resources.cpus().isSome()) {
      cpus = resources.cpus().get();
    } else {
      ...
    }
    
    



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29302>

    ditto not storing it



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29303>

    ditto not storing it



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29304>

    ditto not storing it



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29305>

    stringify can do this work for you:
    
    ports = stringify(resources.ports().get())



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29300>

    I preferred the old format FWIW, looks like this new one conforms less to 
the formatting style we discussed?



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29299>

    Can we keep mem and disk as %d? What does a fractional byte mean?
    
    I guess we should have had an Integer type Scalar, for things like this.



src/slave/slave.cpp
<https://reviews.apache.org/r/8171/#comment29298>

    s/os::usage/fs::usage in your TODO



third_party/libprocess/include/stout/fs.hpp
<https://reviews.apache.org/r/8171/#comment29297>

    s/total/total available



third_party/libprocess/include/stout/fs.hpp
<https://reviews.apache.org/r/8171/#comment29296>

    thanks


- Ben Mahler


On Nov. 21, 2012, 6:47 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8171/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2012, 6:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> Now we properly set cpu/mem/disk/ports based on what's missing in slave flags.
> 
> Also moved usage from os to fs.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 7deb4574943aae4cfc5da5d6b3f600042686975f 
>   src/tests/exception_tests.cpp 13355d08788432ed07679daf24c2d74cc12a7f11 
>   third_party/libprocess/include/stout/fs.hpp 
> 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d 
>   third_party/libprocess/include/stout/os.hpp 
> 76e5e0624af36a0021755fb4acf7f76bfb81a823 
> 
> Diff: https://reviews.apache.org/r/8171/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to