mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Timothy Chen <tnac...@apache.org>
Subject Re: Review Request 43032: Set Docker labels based on TaskInfo labels.
Date Fri, 12 Feb 2016 15:43:30 GMT

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




src/docker/docker.cpp (line 536)
<https://reviews.apache.org/r/43032/#comment180322>

    Seems like we're increasing trying to find docker versions in our code base. How about
let's capture the docker version and save it in the Docker class, so we can re-use it and
don't have to compute it every time?



src/docker/docker.cpp (line 543)
<https://reviews.apache.org/r/43032/#comment180323>

    Does label.value() return a Try? What kind of error do we expect from it?



src/tests/containerizer/docker_tests.cpp (line 132)
<https://reviews.apache.org/r/43032/#comment180324>

    How about formatting like:
    
    i.. = strings::remove(
      strings::remove(i.., "[", strings::PREFIX),
      "]",
      strings::SUFFIX);



src/tests/containerizer/docker_tests.cpp (line 139)
<https://reviews.apache.org/r/43032/#comment180325>

    You need to EXPECT_SOME on the find first, other wise it's basically an assert.


- Timothy Chen


On Feb. 5, 2016, 10:52 a.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2016, 10:52 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4446
>     https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 645bdcf095145097d8b8c65d592c787417883145

>   src/tests/containerizer/docker_tests.cpp f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> -------
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to docker run
command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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