mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chi Zhang" <chzhc...@gmail.com>
Subject Re: Review Request 34142: AppC provisioner.
Date Mon, 18 May 2015 22:20:36 GMT

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



src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/34142/#comment135361>

    can tweak the constructors so that the containerizer doesn't need to be exposed to ProvisionerProcesses,
which is implementation detail?



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135339>

    nit: return type could be const?



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135375>

    If we say the dependency itself "is an" AppcImage that "has n" AppcImage(dependencies).
We can kill the dependency type?
    
    AppcImage could then recursively resolve the dependencies on its own, using the composite
pattern described above, i.e., AppcProvisionerProcess simply needs to call rootImage.resolve(),
who loops over dependent images and calls image.resolve().
    
    We can then have fetch, fetchDependencies (not needed anymore), fetchURIs and match togerther
go into AppcImage::resolve. AppcImage will now need functionality provided by discover and
store; AppcPP only uses backend and does mounts.



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135340>

    const-able?



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135341>

    nit: newline after ','



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135342>

    nit: craft a list and then return "-".join(list)?



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135343>

    some of these should be const?



src/slave/containerizer/provisioners/appc.hpp
<https://reviews.apache.org/r/34142/#comment135358>

    nit: const-able?



src/slave/containerizer/provisioners/appc.cpp
<https://reviews.apache.org/r/34142/#comment135360>

    Is it possible to tweak the store{process}'s constructors to hide the implemention detail
of there existing a StoreProcess?


- Chi Zhang


On May 13, 2015, 12:48 a.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 12:48 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Discovers image(s), fetches to the image store, then provisions using
> a backend.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
>   src/slave/containerizer/mesos/containerizer.cpp b644b9c74bc23cf78c0a53284544be6cdaef2f8a

>   src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
> 
> Diff: https://reviews.apache.org/r/34142/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ian Downes
> 
>


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