----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/#review95831 -----------------------------------------------------------
src/slave/containerizer/provisioners/docker.hpp (line 81) <https://reviews.apache.org/r/37198/#comment150975> Did we introduce DockerImageName later? A pair of strings is pretty confusing, how about pulling it earlier to here? src/slave/containerizer/provisioners/docker.hpp (line 84) <https://reviews.apache.org/r/37198/#comment150976> Actually it's valid to have a registry name with a custom port in the image name: localhost:5050/ubuntu:14.04 You need to first split on "/", then do this check. And we should actually include a optional registry name in the DockerImageName struct too. src/slave/containerizer/provisioners/docker.cpp (line 96) <https://reviews.apache.org/r/37198/#comment150977> I think we usually put all the validation logic inside of the Process so it's all one place, rather than checking here, which makes this object very simple. src/slave/containerizer/provisioners/docker.cpp (line 163) <https://reviews.apache.org/r/37198/#comment150978> Seems like a useless comment :) src/slave/containerizer/provisioners/docker.cpp (line 234) <https://reviews.apache.org/r/37198/#comment150979> "Unable to join discovery local path: " + error() - Timothy Chen On Aug. 17, 2015, 9:11 p.m., Lily Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37198/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2015, 9:11 p.m.) > > > Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, > and Jiang Yan Xu. > > > Bugs: MESOS-2850 > https://issues.apache.org/jira/browse/MESOS-2850 > > > Repository: mesos > > > Description > ------- > > Add Docker image provisioner and copy backend. > > > Diffs > ----- > > src/Makefile.am 457ad26ee55bd7a2aedf27f45db58a9a4a6a5dc5 > src/slave/containerizer/isolators/filesystem/linux.cpp > f36424e94c380870cfde49d55af397fa3dc4a612 > src/slave/containerizer/provisioner.hpp > 541dd4e0b2f0c92a45c00cab6132a2be69654838 > src/slave/containerizer/provisioner.cpp > efc7e6996ff6663bebaf61989a7e040bd2ad7a5e > src/slave/containerizer/provisioners/docker.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker.cpp PRE-CREATION > src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION > src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 > src/slave/flags.cpp e43dd1c13dd4263dc326842233808ddb7a9bb74c > src/tests/containerizer/provisioner.hpp > c4ba46794fe5d7875fda11155367f521c34ea339 > > Diff: https://reviews.apache.org/r/37198/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Lily Chen > >
