----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38580/#review101261 -----------------------------------------------------------
src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 136) <https://reviews.apache.org/r/38580/#comment158634> Replace + with << src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 173) <https://reviews.apache.org/r/38580/#comment158635> Fix identation, and let's just all use << src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 186) <https://reviews.apache.org/r/38580/#comment158636> Does this warrent a LOG(INFO)? Perhaps VLOG(1)? I suspect this is a pretty normal situation. src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 189) <https://reviews.apache.org/r/38580/#comment158637> Don't need to assign this right? Just return _downloadTracker.at(layer.layerId).future() src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 209) <https://reviews.apache.org/r/38580/#comment158638> using namespace process and you can get rid of process:: here, we do that everywhere src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 227) <https://reviews.apache.org/r/38580/#comment158639> I thought you wanted to move this to somewhere shared? We can create a base puller class and move this there. src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 245) <https://reviews.apache.org/r/38580/#comment158640> You need to return here. src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 309) <https://reviews.apache.org/r/38580/#comment158641> This doesn't look right, it's simply a invalid timeout value that's passed to puller, nothing to do with remote name to canonial name src/slave/containerizer/provisioner/docker/remote_puller.cpp (line 317) <https://reviews.apache.org/r/38580/#comment158644> Why break down layer timeout? Why not just one timeout? And how is the user able to configure this pull timeout in the first place? If we don't have give users the option to change this I suggest removing this from the interface and let's just use constants for now. - Timothy Chen On Oct. 1, 2015, 6:40 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38580/ > ----------------------------------------------------------- > > (Updated Oct. 1, 2015, 6:40 p.m.) > > > Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > Integrated remote puller with store. Tests will follow. > > > Diffs > ----- > > src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 > src/slave/containerizer/provisioner/docker/local_puller.hpp > 4574e8a04663482625d7b54f765741f221ec13e0 > src/slave/containerizer/provisioner/docker/local_puller.cpp > 4a0b7d11f013941084571f2d89d835a4668a3d8b > src/slave/containerizer/provisioner/docker/puller.hpp > 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 > src/slave/containerizer/provisioner/docker/puller.cpp > cb05324689ffa26ce830b513e2d71b55517da3cb > src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION > src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION > src/slave/containerizer/provisioner/docker/store.cpp > cbb67686d45513f0395a0cf1bc5c43cb4935adae > src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 > src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 > > Diff: https://reviews.apache.org/r/38580/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
