Re: Review Request 37427: Docker registry: adding TokenManager.

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

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote: > > src/Makefile.am, line 761 > > > > > > Extra space ? > > Jojy Varghese wrote: > looks to be aligned with the rest > > Anand Mazumdar wrote: > The '\'

Re: Review Request 37427: Docker registry: adding TokenManager.

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

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19 > > > > > > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ > > > >

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19 > > > > > > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ > > > >

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 19 > > > > > > Can we change this to be __MESOS_DOCKER_TOKEN_MANAGER_HPP__ > > > >

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 2, 2015, 8:01 p.m., Anand Mazumdar wrote: > > Nothing major, Just some fly-by style comments. Looking at the > > implementation in greater detail now. > > > > I wonder if we should split this review into smaller chunks. It currently > > stands at ~900 lines. It's very hard to review

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review97505 --- Nothing major, Just some fly-by style comments. Looking at the imple

Re: Review Request 37427: Docker registry: adding TokenManager.

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

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 1, 2015, 9:13 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 61 > > > > > > Its effectively the same. const reference as method argument is just a > > g

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review97338 --- src/tests/provisioners/docker_provisioner_tests.cpp (line 341)

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Sept. 1, 2015, 9:13 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 61 > > > > > > Its effectively the same. const reference as method argument is just a > > g

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review97336 --- src/slave/containerizer/provisioners/docker/token_manager.cpp (line

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review97329 --- Ship it! Ship It! src/slave/containerizer/provisioners/docker/tok

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 31, 2015, 10:39 p.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 30, 2015, 3:10 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 28, 2015, 6:36 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 223 > > > > > > Is there a reason you need to include this in the header? Can we just > >

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 28, 2015, 7:56 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 223 > > > > > > If you put the cache here in TokenManager instead of > > TokenManagerProces

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 203 > > > > > > We usually name the class variable without _, and the parameter with _. >

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96841 --- src/slave/containerizer/provisioners/docker/token_manager.cpp (line

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 78 > > > > > > Captialize first word (Invalid), same as other error messages below. > > Joj

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 28, 2015, 4:27 a.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 28, 2015, 12:46 a.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 69 > > > > > > is exp and nbf web token fields? Yes. > On Aug. 28, 2015, 12:18 a.m., Timo

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96752 --- src/slave/containerizer/provisioners/docker/token_manager.hpp (line

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96685 --- src/tests/provisioners/docker_provisioner_tests.cpp (lines 52 - 82)

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 25, 2015, 10:11 p.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 318 > > > > > > Capitalize beginning of failure messages. for internal error messages (that b

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 26, 2015, 1:03 a.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96421 --- src/Makefile.am (line 477)

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96233 --- Patch looks great! Reviews applied: [37426, 37427] All tests passe

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 24, 2015, 5:16 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95969 --- Patch looks great! Reviews applied: [37426, 37427] All tests passe

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 20, 2015, 3:50 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95898 --- Bad patch! Reviews applied: [37426, 37427] Failed command: ./suppo

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 19, 2015, 5:25 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 18, 2015, 11:24 p.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95457 --- Patch looks great! Reviews applied: [37426, 37427] All tests passe

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 14, 2015, 5:28 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 14, 2015, 2:43 p.m.) Review request for mesos, Lily Chen, Joris V

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 14, 2015, 12:09 a.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 10:26 p.m.) Review request for mesos, Lily Chen, Joris

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95326 --- src/slave/containerizer/provisioners/docker/token_manager.hpp (line

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 13, 2015, 6:24 a.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 252 > > > > > > space between () {}, you could also probably just do this in the header > > fil

Re: Review Request 37427: Docker registry: adding TokenManager.

> On Aug. 13, 2015, 6:24 a.m., Lily Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, lines 37-46 > > > > > > no need to put underscore in front of parameters being passed in As per style

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95243 --- Patch looks great! Reviews applied: [37426, 37427] All tests passe

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 8:29 a.m.) Review request for mesos, Lily Chen and Timo

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95238 --- src/slave/containerizer/provisioners/docker/token_manager.hpp (line

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 4:47 a.m.) Review request for mesos, Lily Chen and Timo

Re: Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95230 --- Bad patch! Reviews applied: [37426, 37427] Failed command: ./suppo

Review Request 37427: Docker registry: adding TokenManager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- Review request for mesos, Lily Chen and Timothy Chen. Repository: mesos Descr