This is an automatically generated e-mail. To reply, visit:

Thanks Klaus, this is great! It will be awesome to have an example framework 
for dynamic reservation :-)

src/Makefile.am (line 1600)

    It looks like hyphens rather than underscores here would be more consistent 
with the naming of other example frameworks: `dynamic-reservation-framework`.

src/examples/dynamic_reservation_framework.cpp (line 27)

    The ACL definitions were recently moved into 
include/mesos/authorizer/acls.hpp, so this include needs to be changed.

src/examples/dynamic_reservation_framework.cpp (line 42)

    Instead of using boost here, perhaps we could use members of the STL like 
`std::stoi` and `std::to_string`?

src/examples/dynamic_reservation_framework.cpp (line 56)

    Is this true? It looks like we will launch multiple tasks on some slaves.

src/examples/dynamic_reservation_framework.cpp (lines 57 - 58)

    s/when it's offered to framework at the first time/when offered to the 
framework for the first time/
    s/tasks done/tasks are done/

src/examples/dynamic_reservation_framework.cpp (line 84)

    Would you mind changing the logging messages in this file to use the glog 

src/examples/dynamic_reservation_framework.cpp (line 101)

    If you want, you could use `hashmap` for `states` and take advantage of the 
`contains` method here.

src/examples/dynamic_reservation_framework.cpp (line 117)

    s/reserved resources re-offer/reserved resources are re-offered/

src/examples/dynamic_reservation_framework.cpp (line 130)

    s/this resources/these resources/

src/examples/dynamic_reservation_framework.cpp (line 143)

    s/waiting for task done/waiting for task to finish/
    Also, should remove the period from the end of the logging message

src/examples/dynamic_reservation_framework.cpp (line 157)

    s/tasks done/tasks are done/
    s/resources unreserved/resources are unreserved/

src/examples/dynamic_reservation_framework.cpp (line 207)

    s/for unreserving resources/for resources to be unreserved/
    Also, remove period at end

src/examples/dynamic_reservation_framework.cpp (line 255)

    s/Fail to/Failed to/
    Also, remove period at end

src/examples/dynamic_reservation_framework.cpp (line 423)

    This error message could be a bit more descriptive, something like: "The 
default '*' role cannot be used"
    Also, the period at the end of the error message should be removed.

src/examples/dynamic_reservation_framework.cpp (line 429)

    Should this comment be a TODO?

src/examples/dynamic_reservation_framework.cpp (line 441)

    Now that we have implicit roles, I don't think this is necessary?

- Greg Mann

On March 7, 2016, 9:20 a.m., Klaus Ma wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> -----------------------------------------------------------
> (Updated March 7, 2016, 9:20 a.m.)
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> Bugs: MESOS-3063
>     https://issues.apache.org/jira/browse/MESOS-3063
> Repository: mesos
> Description
> -------
> Provide example for dynamic reservation features.
> Diffs
> -----
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> Diff: https://reviews.apache.org/r/37168/diff/
> Testing
> -------
> make
> make check
> Thanks,
> Klaus Ma

Reply via email to