mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 37168: Add an example framework using dynamic reservation.
Date Tue, 15 Mar 2016 17:40:03 GMT

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



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


src/Makefile.am (line 1600)
<https://reviews.apache.org/r/37168/#comment184266>

    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)
<https://reviews.apache.org/r/37168/#comment185894>

    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)
<https://reviews.apache.org/r/37168/#comment185940>

    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)
<https://reviews.apache.org/r/37168/#comment185943>

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



src/examples/dynamic_reservation_framework.cpp (lines 57 - 58)
<https://reviews.apache.org/r/37168/#comment185901>

    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)
<https://reviews.apache.org/r/37168/#comment185933>

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



src/examples/dynamic_reservation_framework.cpp (line 101)
<https://reviews.apache.org/r/37168/#comment185917>

    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)
<https://reviews.apache.org/r/37168/#comment185920>

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



src/examples/dynamic_reservation_framework.cpp (line 130)
<https://reviews.apache.org/r/37168/#comment185924>

    s/this resources/these resources/



src/examples/dynamic_reservation_framework.cpp (line 143)
<https://reviews.apache.org/r/37168/#comment185929>

    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)
<https://reviews.apache.org/r/37168/#comment185934>

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



src/examples/dynamic_reservation_framework.cpp (line 207)
<https://reviews.apache.org/r/37168/#comment185930>

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



src/examples/dynamic_reservation_framework.cpp (line 255)
<https://reviews.apache.org/r/37168/#comment185931>

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



src/examples/dynamic_reservation_framework.cpp (line 423)
<https://reviews.apache.org/r/37168/#comment185936>

    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)
<https://reviews.apache.org/r/37168/#comment185935>

    Should this comment be a TODO?



src/examples/dynamic_reservation_framework.cpp (line 441)
<https://reviews.apache.org/r/37168/#comment185937>

    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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message