> 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
> 
>

Reply via email to