----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56174/#review164257 -----------------------------------------------------------
Fix it, then Ship it! I'll commit the code first without the test. Please follow up with the test using my suggestion! Thanks! src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp (lines 287 - 288) <https://reviews.apache.org/r/56174/#comment235908> I typically wrap comments in 70 char width. src/tests/containerizer/provisioner_docker_tests.cpp (lines 262 - 270) <https://reviews.apache.org/r/56174/#comment235909> Instead of testing local puller directly, I'd suggest we test using 'Store'. This test assumes too many impl details which will be very fragile as code evolves. We try to avoid that if possible. My suggestion is that we pull the image once, making sure store has it. And then, delete one layer, and then pull again. Make sure the second pull is successful. - Jie Yu On Feb. 3, 2017, 6:07 p.m., Ilya Pronin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56174/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2017, 6:07 p.m.) > > > Review request for mesos, Jie Yu and Timothy Chen. > > > Bugs: MESOS-7045 > https://issues.apache.org/jira/browse/MESOS-7045 > > > Repository: mesos > > > Description > ------- > > If a layer is already in the store, there's no need to extract it. > `RegistryPuller` already does this and `Store` is ready for such puller > behaviour. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp > ee391af898886bff9e5b911697f725c5ea53ebd8 > src/tests/containerizer/provisioner_docker_tests.cpp > af9987f88205d00d091f35fa734d5667506aaffd > > Diff: https://reviews.apache.org/r/56174/diff/ > > > Testing > ------- > > Added a test verifying that local puller will skip layes that are already in > the store. Ran `make check`. > > > Thanks, > > Ilya Pronin > >
