mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.
Date Mon, 16 Jul 2018 23:07:09 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67931/#review206134
-----------------------------------------------------------



Ah, sorry, I will get the rest of this reviewed tomorrow.


3rdparty/cmake/Versions.cmake
Lines 7-8 (original), 7-8 (patched)
<https://reviews.apache.org/r/67931/#comment289034>

    It looks like cURL has since posted 7.61.0, so as long as we're updating this, let's update
to 7.61.0, and get it added to https://github.com/mesos/3rdparty/ so we can get the ReviewBot
to pass. (We spoke offine so I know you're already doing this, just posting it for posterity.)



src/CMakeLists.txt
Lines 170 (patched)
<https://reviews.apache.org/r/67931/#comment289038>

    I'm not certain we can just add it to `AGENT_SRC` as that'll also add it for POSIX systems.
Instead, like how it's added to `LINUX_SRC`, let's also add it to `WIN32_SRC`.
    
    I would suggest, take this existing snippet in this file (which shouldn't exist given
the existence of `WIN32_SRC`:
    
    ```
    if (WIN32)
      list(APPEND AGENT_SRC
        slave/containerizer/mesos/isolators/windows/cpu.cpp
        slave/containerizer/mesos/isolators/windows/mem.cpp)
    else ()
    ```
    
    and instead make:
    
    ```
    set(WIN32)SRC
      slave/containerizer/mesos/isolators/docker/runtime.cpp
      slave/containerizer/mesos/isolators/windows/cpu.cpp
      slave/containerizer/mesos/isolators/windows/mem.cpp
      ... <the other three files in this variable already>)
    ```



src/CMakeLists.txt
Lines 424-427 (original), 426-427 (patched)
<https://reviews.apache.org/r/67931/#comment289039>

    With this being true, we can just move `uri/fetchers/docker.cpp` to the point where we're
setting `URI_SRC` to begin with.



src/uri/fetchers/docker.cpp
Lines 102-105 (patched)
<https://reviews.apache.org/r/67931/#comment289041>

    I think there's a `strings::replace` to do this.


- Andrew Schwartzmeyer


On July 16, 2018, 11:35 a.m., Liangyu Zhao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> -----------------------------------------------------------
> 
> (Updated July 16, 2018, 11:35 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.60.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a

>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp a5683e3fe15dd35596122fcc0c580ae9d3adf7f2

>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>


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