mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 43855: Added Appc fetcher support to store.
Date Fri, 26 Feb 2016 05:37:00 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message