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

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.

> 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

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

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

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

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

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

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 https://reviews.apache.org/r/37427/diff/12/?file=1052806#file1052806line223 Is there a reason you need to include this in the header? Can we just put it in

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 https://reviews.apache.org/r/37427/diff/14/?file=1057180#file1057180line223 If you put the cache here in TokenManager instead of TokenManagerProcess then

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

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 https://reviews.apache.org/r/37427/diff/12/?file=1052806#file1052806line69 is exp and nbf web token fields? Yes. On Aug. 28, 2015, 12:18 a.m., Timothy Chen

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/ --- (Updated Aug. 24, 2015, 5:16 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/#review96233 --- Patch looks great! Reviews applied: [37426, 37427] All tests

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

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

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:

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/ --- (Updated Aug. 14, 2015, 2:43 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. 14, 2015, 5:28 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

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

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

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

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 https://reviews.apache.org/r/37427/diff/2/?file=1039212#file1039212line37 no need to put underscore in front of parameters being passed in As per style docs, its

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/ --- (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/#review95326 --- src/slave/containerizer/provisioners/docker/token_manager.hpp

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 https://reviews.apache.org/r/37427/diff/2/?file=1039212#file1039212line252 space between () {}, you could also probably just do this in the header file 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/#review95230 --- Bad patch! Reviews applied: [37426, 37427] Failed command:

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

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