> 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 wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 77 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052806#file1052806line77> > > > > Why keep raw? Raw is what is used by docker client. Other fields are useful for validation etc. > 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 cpp? The only reason being that this is a property of the TokenManager. > On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 78 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052807#file1052807line78> > > > > Captialize first word (Invalid), same as other error messages below. for internal error messages (that bubbles up), the leaf level messsages are recommended to start with lower case. This is so that when the root level logger logs the message, it does not look like : "Error to do operation foo: Failed to fetch data". > On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 165 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052807#file1052807line165> > > > > Shouldn't this return true? No if code does not follow !expired, then the token must be expired and hence invalid. > On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 203 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052807#file1052807line203> > > > > We usually name the class variable without _, and the parameter with _. Not sure I follow. > On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 164 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052807#file1052807line164> > > > > Kill space Will fix. - Jojy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96752 ----------------------------------------------------------- On Aug. 26, 2015, 1:03 a.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > 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 Van Remoortere, and Timothy Chen. > > > Repository: mesos > > > Description > ------- > > Changes: > - Added Token implementation (RFC 7519). > - Added TokenManager implementation. This component keeps a cache of tokens > requested for any future requests. > > > Diffs > ----- > > src/Makefile.am 571e1ac0f96b2452797a478680b540f2aab63aab > src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION > src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/37427/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
