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