mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 37427: Docker registry: adding TokenManager.
Date Fri, 28 Aug 2015 07:49:10 GMT


> 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.
> 
> Jojy Varghese wrote:
>     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".

We don't really follow this practice in Mesos, so if you feel like this is something we should
do I think you can propose and we can discuss about this.
And you never know who is nesting your error message anyways, so you could also end up "Error
abc: Error to do op foo: failed to def". For now let's just be consistent with all the code
base.


> 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?
> 
> Jojy Varghese wrote:
>     The only reason being that this is a property of the TokenManager.

I see, does it need to be? Can't we define a static const in the cpp?


- Timothy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37427/#review96752
-----------------------------------------------------------


On Aug. 28, 2015, 4:27 a.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> 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 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 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message