> On Feb. 13, 2016, 1:58 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp, line 89 > > <https://reviews.apache.org/r/43336/diff/4/?file=1241021#file1241021line89> > > > > No need for 'doFetchImage' since only this function is using it. Can > > you just inline it here?
The idea was that this method will remain common for all discovery types. > On Feb. 13, 2016, 1:58 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp, line 93 > > <https://reviews.apache.org/r/43336/diff/4/?file=1241021#file1241021line93> > > > > No need for 'const' here as it'll be file local method. The idea was that all discovery related methods like this would be class methods because the fetcher is after all appc discovery spec implementation. > On Feb. 13, 2016, 1:58 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp, lines 207-212 > > <https://reviews.apache.org/r/43336/diff/4/?file=1241021#file1241021line207> > > > > Hum, do you need to mkdir? will untar create it? No when i tested with rkt image (coreos/etcd2), after decompressin when i untar the tar file, i will untar without creating a directory. > On Feb. 13, 2016, 1:58 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp, line 214 > > <https://reviews.apache.org/r/43336/diff/4/?file=1241021#file1241021line214> > > > > Do you need to remove aciBundle? > > > > ``` > > .then([]() -> Future<Nothing> { > > return os::rm(...); > > }); > > ``` I could but I thought we agreed that this is a temp directory which will be deleted by the caller. The caller would do 'ls' in this directory and pick only the directories. - Jojy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43336/#review119123 ----------------------------------------------------------- On Feb. 13, 2016, 1:43 a.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43336/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2016, 1:43 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4596 > https://issues.apache.org/jira/browse/MESOS-4596 > > > Repository: mesos > > > Description > ------- > > Added implementation for simple image discovery for Appc images. > > TODO: Add tests. > > > Diffs > ----- > > src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 > src/Makefile.am 5813ab2c33a7de6b612064e894e5f15b5a474e2b > src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION > src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION > src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d > src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 > > Diff: https://reviews.apache.org/r/43336/diff/ > > > Testing > ------- > > Tested with local http(s) server. > > > Thanks, > > Jojy Varghese > >
