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