Re: Review Request 37773: Docker: Adding registry client.

2015-09-10 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review98368 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 10, 2015, 4:53 a.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 10, 2015, 4:49 a.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review98347 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review98313 --- src/slave/containerizer/provisioners/docker/registry_client.hpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review98312 --- docs/persistent-volume.md (line 227)

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 9, 2015, 11:47 p.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review98308 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 9, 2015, 8:08 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
> On Sept. 1, 2015, 9:35 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 359 > > > > > > Should we make sure somewhere that we encode or check the tag so that > >

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Timothy Chen
> On Sept. 1, 2015, 9:35 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 359 > > > > > > Should we make sure somewhere that we encode or check the tag so that > >

Re: Review Request 37773: Docker: Adding registry client.

2015-09-09 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 9, 2015, 4:50 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 2, 2015, 10:52 p.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97518 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 2, 2015, 5:06 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37773: Docker: Adding registry client.

2015-09-02 Thread Jojy Varghese
- Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97331 --- On Sept. 1, 2015, 9:36 p.m., Jojy Varghese wrote: > > ---

Re: Review Request 37773: Docker: Adding registry client.

2015-09-01 Thread Jojy Varghese
> On Sept. 1, 2015, 9:35 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 410 > > > > > > I think ideally we can introduce something in libprocess so we can > > str

Re: Review Request 37773: Docker: Adding registry client.

2015-09-01 Thread Jojy Varghese
> On Sept. 1, 2015, 9:35 p.m., Timothy Chen wrote: > > src/tests/provisioners/docker_provisioner_tests.cpp, line 232 > > > > > > Why is this needed? So that we cleanup temporaries created in the SSLTest setup. - J

Re: Review Request 37773: Docker: Adding registry client.

2015-09-01 Thread Jojy Varghese
> On Sept. 1, 2015, 9:35 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 186 > > > > > > What's the benefits for this inline lambda vs just putting this code in >

Re: Review Request 37773: Docker: Adding registry client.

2015-09-01 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97347 --- src/slave/containerizer/provisioners/docker/registry_client.cpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-09-01 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97331 --- src/slave/containerizer/provisioners/docker/registry_client.hpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-31 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97235 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-08-31 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Sept. 1, 2015, 12:02 a.m.) Review request for mesos, Lily Chen and Tim

Re: Review Request 37773: Docker: Adding registry client.

2015-08-31 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 31, 2015, 10:40 p.m.) Review request for mesos, Lily Chen and Tim

Re: Review Request 37773: Docker: Adding registry client.

2015-08-31 Thread Jojy Varghese
> On Aug. 31, 2015, 8:42 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 456 > > > > > > is maxSize being used at all? If not we should remove all references of >

Re: Review Request 37773: Docker: Adding registry client.

2015-08-31 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97138 --- src/slave/containerizer/provisioners/docker/registry_client.hpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-30 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review97029 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-08-30 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 30, 2015, 3:12 p.m.) Review request for mesos, Lily Chen and Timo

Re: Review Request 37773: Docker: Adding registry client.

2015-08-30 Thread Jojy Varghese
> On Aug. 28, 2015, 6:48 p.m., Timothy Chen wrote: > > I have a feeling we can remove all the promises usage in this review. Let's > > chat offline. Conceptually an async operation caller creates a promise and returns that promise's future. It then does the async operation which sets the futur

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
> On Aug. 28, 2015, 11:03 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 465 > > > > > > Same issue with path naming as with getManifest Please see https://docs.dock

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
> On Aug. 28, 2015, 6:46 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.hpp, line 150 > > > > > > Do we need to define static here if it's not being used anywhere else > >

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
> On Aug. 28, 2015, 11:03 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 409 > > > > > > I think path may be misleading, can we name this repo? https://docs.docker.c

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
> On Aug. 28, 2015, 11:03 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.cpp, line 415 > > > > > > can we do a path::join here? It would be hard to get the "/"s correct > > ju

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96957 --- src/slave/containerizer/provisioners/docker/registry_client.cpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Lily Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96932 --- src/slave/containerizer/provisioners/docker/registry_client.cpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96908 --- Patch looks great! Reviews applied: [37871, 37427, 37773] All test

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96907 --- I have a feeling we can remove all the promises usage in this review

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96905 --- src/slave/containerizer/provisioners/docker/registry_client.hpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 28, 2015, 6:38 p.m.) Review request for mesos, Lily Chen and Timo

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Jojy Varghese
> On Aug. 28, 2015, 5:21 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.hpp, line 34 > > > > > > No need for provisioners namespace, see appc example of > > mesos::internal::sl

Re: Review Request 37773: Docker: Adding registry client.

2015-08-28 Thread Lily Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96896 --- src/slave/containerizer/provisioners/docker/registry_client.hpp (li

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96830 --- Bad patch! Reviews applied: [37871, 37427, 37773] Failed command:

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 28, 2015, 4:26 a.m.) Review request for mesos, Lily Chen and Timo

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96813 --- Bad patch! Reviews applied: [37426, 37427] Failed command: ./suppo

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 28, 2015, 12:47 a.m.) Review request for mesos, Lily Chen and Tim

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Jojy Varghese
> On Aug. 26, 2015, midnight, Lily Chen wrote: > > This was a WIP for you to get the patch :). Anywaz addressed the issues. > On Aug. 26, 2015, midnight, Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/registry_client.hpp, line 89 > >

Re: Review Request 37773: Docker: Adding registry client.

2015-08-27 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/ --- (Updated Aug. 28, 2015, 12:03 a.m.) Review request for mesos, Lily Chen and Tim