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

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

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 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 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

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

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 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. 9, 2015, 4:50 p.m.) Review request for mesos, Lily Chen, Joris

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, 8:08 p.m.) Review request for mesos, Lily Chen, Joris

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-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

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-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

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-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. -

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

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

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 > >

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-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

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

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 https://reviews.apache.org/r/37773/diff/5/?file=1057183#file1057183line34 No need for provisioners namespace, see appc example of mesos::internal::slave::appc

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

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 https://reviews.apache.org/r/37773/diff/6/?file=1058283#file1058283line415 can we do a path::join here? It would be hard to get the /s correct just passing

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 https://reviews.apache.org/r/37773/diff/6/?file=1058283#file1058283line409 I think path may be misleading, can we name this repo?

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 https://reviews.apache.org/r/37773/diff/6/?file=1058282#file1058282line150 Do we need to define static here if it's not being used anywhere else but the cpp

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:

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:47 a.m.) Review request for mesos, Lily Chen and