----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/#review120828 -----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 254) <https://reviews.apache.org/r/43855/#comment182336> Why we need this parameter? What does that mean? To me, it's clear that 'fetchImage' just take 'appc'. src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 267) <https://reviews.apache.org/r/43855/#comment182337> Why do you need to call validate? Any image in the store should already been validated. You do have to check if the image id exists in the store or not (cache->get(...)?) src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 278) <https://reviews.apache.org/r/43855/#comment182345> I would put this to the end, so the overall logic of this method should be look like the following: 1) see if the image id in the cache or not 2) if not, fetch it using fetcher 3) once fetch is done (i.e., moved to the store), removed the staging dir 4) fetch dependencies, get a vector of image ids 5) merge the ids get from (4) and merge it with the image id get from either 1) or 3) src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 298 - 323) <https://reviews.apache.org/r/43855/#comment182346> Any reason you need three lambdas? Looks like they are synchronous functions. - Jie Yu On Feb. 26, 2016, 4:07 a.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43855/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2016, 4:07 a.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > ------- > > This change allows store to fetch an image using Appc image fetcher when an > image is not found in the cache. It also recursively fetches the dependencies > for the image. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/appc/store.cpp > 4b3829175f57fb9aea2478040d96f2f127cbc551 > src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e > > Diff: https://reviews.apache.org/r/43855/diff/ > > > Testing > ------- > > make check; local image server. > > > Thanks, > > Jojy Varghese > >
