You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Sean Busbey <se...@manvsbeard.com> on 2014/04/25 09:07:13 UTC
Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2733
https://issues.apache.org/jira/browse/ACCUMULO-2733
Repository: accumulo
Description
-------
adds fromThrift to Credentials, consolidates existing one-off deserializations.
If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
Diffs
-----
core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
Diff: https://reviews.apache.org/r/20702/diff/
Testing
-------
unit tests passed. ITs running.
Thanks,
Sean Busbey
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/#review41471
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Havanki
On April 25, 2014, 10:53 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20702/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 10:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2733
> https://issues.apache.org/jira/browse/ACCUMULO-2733
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> adds fromThrift to Credentials, consolidates existing one-off deserializations.
>
> If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
> core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
> server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
> server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
>
> Diff: https://reviews.apache.org/r/20702/diff/
>
>
> Testing
> -------
>
> unit tests passed. ITs passed. No new warnings show up via build.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/#review41469
-----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/security/Credentials.java
<https://reviews.apache.org/r/20702/#comment74900>
"tehe" - totally just fix this on commit
- Bill Havanki
On April 25, 2014, 10:53 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20702/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 10:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2733
> https://issues.apache.org/jira/browse/ACCUMULO-2733
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> adds fromThrift to Credentials, consolidates existing one-off deserializations.
>
> If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
> core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
> server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
> server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
>
> Diff: https://reviews.apache.org/r/20702/diff/
>
>
> Testing
> -------
>
> unit tests passed. ITs passed. No new warnings show up via build.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/
-----------------------------------------------------------
(Updated April 25, 2014, 2:53 p.m.)
Review request for accumulo.
Changes
-------
fixed javadoc
Bugs: ACCUMULO-2733
https://issues.apache.org/jira/browse/ACCUMULO-2733
Repository: accumulo
Description
-------
adds fromThrift to Credentials, consolidates existing one-off deserializations.
If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
Diffs (updated)
-----
core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
Diff: https://reviews.apache.org/r/20702/diff/
Testing (updated)
-------
unit tests passed. ITs passed. No new warnings show up via build.
Thanks,
Sean Busbey
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Sean Busbey <se...@manvsbeard.com>.
> On April 25, 2014, 1:36 p.m., Bill Havanki wrote:
> > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java, line 201
> > <https://reviews.apache.org/r/20702/diff/1/?file=568326#file568326line201>
> >
> > This check for whether the token class is valid according to the current authenticator appears to be gone. (Maybe it's not needed and/or this has already been discussed.)
It's not needed. Authenticator's need to validate that they can handle AuthenticationTokens passed to them in any case (we did not do this check before all authenticateUser calls). A reimplementation of this check is the first thing the ZKAuthenticator we ship does.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/#review41460
-----------------------------------------------------------
On April 25, 2014, 7:07 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20702/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 7:07 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2733
> https://issues.apache.org/jira/browse/ACCUMULO-2733
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> adds fromThrift to Credentials, consolidates existing one-off deserializations.
>
> If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
> core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
> server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
> server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
>
> Diff: https://reviews.apache.org/r/20702/diff/
>
>
> Testing
> -------
>
> unit tests passed. ITs running.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Sean Busbey <se...@manvsbeard.com>.
> On April 25, 2014, 1:36 p.m., Bill Havanki wrote:
> > core/src/main/java/org/apache/accumulo/core/security/Credentials.java, line 94
> > <https://reviews.apache.org/r/20702/diff/1/?file=568323#file568323line94>
> >
> > Nit: add @param and @return
fixed
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/#review41460
-----------------------------------------------------------
On April 25, 2014, 7:07 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20702/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 7:07 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2733
> https://issues.apache.org/jira/browse/ACCUMULO-2733
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> adds fromThrift to Credentials, consolidates existing one-off deserializations.
>
> If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
> core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
> server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
> server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
>
> Diff: https://reviews.apache.org/r/20702/diff/
>
>
> Testing
> -------
>
> unit tests passed. ITs running.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20702: ACCUMULO-2733 Adds fromThrift to Credentials
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20702/#review41460
-----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/security/Credentials.java
<https://reviews.apache.org/r/20702/#comment74892>
Nit: add @param and @return
server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
<https://reviews.apache.org/r/20702/#comment74893>
This check for whether the token class is valid according to the current authenticator appears to be gone. (Maybe it's not needed and/or this has already been discussed.)
- Bill Havanki
On April 25, 2014, 3:07 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20702/
> -----------------------------------------------------------
>
> (Updated April 25, 2014, 3:07 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2733
> https://issues.apache.org/jira/browse/ACCUMULO-2733
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> adds fromThrift to Credentials, consolidates existing one-off deserializations.
>
> If ACCUMULO-2726 goes in first, I'll refactor this patch to just do the consolidation.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/security/Credentials.java 5afc6e8
> core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java 4f8079e
> server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java 3571d7f
> server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java c2a7001
>
> Diff: https://reviews.apache.org/r/20702/diff/
>
>
> Testing
> -------
>
> unit tests passed. ITs running.
>
>
> Thanks,
>
> Sean Busbey
>
>