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



src/slave/containerizer/provisioner.cpp (line 27)
<https://reviews.apache.org/r/37881/#comment152581>

    See my comments below. This is no longer needed.



src/slave/containerizer/provisioner.cpp (lines 43 - 46)
<https://reviews.apache.org/r/37881/#comment152580>

    I would love to get this TODO solved in this patch. It should be pretty 
straightfoward, right? Just hard code them. And right now, only Appc is 
supported.



src/slave/containerizer/provisioners/appc.cpp (lines 79 - 97)
<https://reviews.apache.org/r/37881/#comment152598>

    Some high level comments.
    
    I think it will be beneficial if we can move the fetch and discovery logic 
to the Store. For the MVP, we don't have to impl. that in the Store.
    
    The Store only has one interface, which is the 'get' method. 'get' takes an 
Image::Appc and returns a vector of paths to layers. THis semantics is more 
natural and easy to understand: get layers for my image, if some layers are 
already cached, return them directly, otherwise, discover it and fetch it.
    
    The job of the provisioner is to get layers from the Store and stack them 
into a rootfs using the backend. It also manages the rootfs directories.
    
    To me, this is a more natural split of functionalities among components. In 
the future, we can also potentially unify the impl. of the provisioner (i.e., 
one single provisioner for both Docker and Appc images). All the image specific 
details are hiden in the Store interface.
    
    Thoughts?



src/slave/flags.hpp (line 53)
<https://reviews.apache.org/r/37881/#comment152583>

    See my comments below. Why you still need this flag?



src/slave/flags.cpp (lines 72 - 76)
<https://reviews.apache.org/r/37881/#comment152582>

    Why do you still need this flag?


- Jie Yu


On Aug. 28, 2015, 9:04 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37881/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 9:04 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-2796
>     https://issues.apache.org/jira/browse/MESOS-2796
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented AppcProvisioner.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioner.cpp 
> efc7e6996ff6663bebaf61989a7e040bd2ad7a5e 
>   src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/paths.hpp 
> 41e3bf79da0854406c488855f953111e67353829 
>   src/slave/containerizer/provisioners/appc/paths.cpp 
> 3113c84b9526dd9e9e89fb9aa4ec75ed66a996c7 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37881/diff/
> 
> 
> Testing
> -------
> 
> sudo make check.
> 
> More test cases coming.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to