mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jiang Yan Xu" <...@jxu.me>
Subject Re: Review Request 34137: Add support for container image provisioners.
Date Mon, 29 Jun 2015 19:41:16 GMT

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



src/slave/containerizer/mesos/containerizer.hpp (lines 319 - 320)
<https://reviews.apache.org/r/34137/#comment142551>

    Why not store ContainerInfo directly?



src/slave/containerizer/mesos/containerizer.cpp (lines 642 - 646)
<https://reviews.apache.org/r/34137/#comment142560>

    Indent two more spaces.



src/slave/containerizer/mesos/containerizer.cpp (lines 789 - 790)
<https://reviews.apache.org/r/34137/#comment142225>

    ```
    map<string, string> env = executorEnvironment(
        executorInfo,
        containers_[containerId]->rootfs.isSome()
          ? flags.sandbox_directory
          : directory,
        slaveId,
        slavePid,
        checkpoint,
        flags.recovery_timeout);
    ```
    ?
    
    Just a comment, not an issue.



src/slave/containerizer/mesos/containerizer.cpp (lines 815 - 816)
<https://reviews.apache.org/r/34137/#comment142224>

    Does
    
    ```
    containers_[containerId]->rootfs.isSome()
      ? flags.sandbox_directory
      : directory;
    ```
    
    look better?
    Just thought this boarders "too much jaggedness" in the sytle guilde.



src/slave/containerizer/mesos/containerizer.cpp (lines 1254 - 1259)
<https://reviews.apache.org/r/34137/#comment142213>

    Shift one more space to the right for alignment.



src/slave/containerizer/mesos/containerizer.cpp (lines 1278 - 1280)
<https://reviews.apache.org/r/34137/#comment142214>

    Four space indentation.



src/slave/containerizer/mesos/containerizer.cpp (line 1279)
<https://reviews.apache.org/r/34137/#comment142223>

    s/" :"/": "/



src/slave/containerizer/provisioner.cpp (lines 46 - 47)
<https://reviews.apache.org/r/34137/#comment142194>

    Seems like this belongs to a later patch. AppcProvisioner is not introduced yet.



src/slave/paths.cpp (lines 243 - 247)
<https://reviews.apache.org/r/34137/#comment141883>

    Indent two more spaces.



src/tests/containerizer_tests.cpp (line 344)
<https://reviews.apache.org/r/34137/#comment142215>

    Kill empty line.


- Jiang Yan Xu


On June 22, 2015, 9:44 a.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> -----------------------------------------------------------
> 
> (Updated June 22, 2015, 9:44 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The MesosContainerizer can optionally provision a root filesystem for the containers.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 3ac2387eabded61c897a5016655aae10cd1bca91

>   src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712

>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ian Downes
> 
>


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