mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Downes" <ian.dow...@gmail.com>
Subject Re: Review Request 34142: AppC provisioner.
Date Sat, 20 Jun 2015 00:49:21 GMT


> On May 18, 2015, 3:20 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc.hpp, line 50
> > <https://reviews.apache.org/r/34142/diff/1/?file=957287#file957287line50>
> >
> >     nit: return type could be const?

Why? I don't see what it would add?


> On May 18, 2015, 3:20 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc.hpp, lines 60-62
> > <https://reviews.apache.org/r/34142/diff/1/?file=957287#file957287line60>
> >
> >     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.

The distinction is that AppcImage::Dependency is something that is desired, and is specified
by a name plus possibly labels and a hash, but has not been realized (fetched). An AppcImage
only refers to an actual image that has been fetched and its properties come from the actual
image data (extracted from the json manifest and or by computing the hash).

AppcImage(::Dependency) is meant to be a simple data structure, the coordination of the discovery,
fetch and store is coordinated by the provisioner.


> On May 18, 2015, 3:20 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc.hpp, line 101
> > <https://reviews.apache.org/r/34142/diff/1/?file=957287#file957287line101>
> >
> >     nit: newline after ','

std::string, std::string are the template parameters for the hashmap, i.e., the whole thing
forms the type for labels.


> On May 18, 2015, 3:20 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc.hpp, lines 74-75
> > <https://reviews.apache.org/r/34142/diff/1/?file=957287#file957287line74>
> >
> >     const-able?

Code refactored to remove this.


> On May 18, 2015, 3:20 p.m., Chi Zhang wrote:
> > src/slave/containerizer/provisioners/appc.hpp, lines 135-139
> > <https://reviews.apache.org/r/34142/diff/1/?file=957287#file957287line135>
> >
> >     some of these should be const?

Code refactored to remove this.


- Ian


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


On May 12, 2015, 5:48 p.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 5:48 p.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