You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2021/01/11 19:35:45 UTC

[kudu-CR] [java] Add JAAS example to client API doc

Hello Alexey Serbin, Andrew Wong, Grant Henke,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/16940

to review the following change.


Change subject: [java] Add JAAS example to client API doc
......................................................................

[java] Add JAAS example to client API doc

The AsyncKuduClient class documents how to connect to a secure Kudu
cluster using ticket cache or using a keytab when the application is
using hadoop-common libraries. We can't always rely on
UserGroupInformation though, so this commit expands the keytab-based
login case by a JAAS config example.

It also fixes a formatting issue with the "@Override" in the code, as it
wasn't properly rendered.

Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 34 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/16940/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@183
PS3, Line 183:  *   com.sun.security.auth.module.Krb5LoginModule required
             :  *   useKeyTab = true
             :  *   keyTab = "/path/to/app.keytab"
> Yea this is the official syntax. Only the last line requires a semi-colon.
Cool, thank you for clarifying.  I asked because this document provides a structure for a file where every line ends with a semi-colon: 

https://docs.oracle.com/en/java/javase/11/security/appendix-b-jaas-login-configuration-file.html



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 20:33:21 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 1:

(3 comments)

Thanks for putting this change together!

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@201
PS1, Line 201: conntect
nit: connect

Alternatively, "Create the KuduClient" or somesuch? Or maybe copy the actual builder code from below? Might not be clear what "connect" means, since there's no public "connect" call.


http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@211
PS1, Line 211: my-ap
nit: not related to this patch, but how about naming this 'app-user' or something so it's clear it's a user. Should be obvious from the UGI docs, but still probably worth clearer naming.

Better yet, it'd be great if both of these examples used consistent user/principals. E.g. point the JAAS example to /path/to/app.keytab and use feasible example users/principals.


http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@220
PS1, Line 220: will also automatically
             :  * start a thread to periodically re-login from the keytab.
Is anything equivalent done for JAAS? Do JAAS users need to worry about doing this themselves?



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 01:49:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has removed a vote on this change.

Change subject: [java] Add JAAS example to client API doc
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3: Code-Review+1

(3 comments)

Just a couple more nits

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@195
PS3, Line 195: n 
nit: add a comma


http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@201
PS3, Line 201:  *   KuduClient client = myClient
I'm curious, can we declare 'client' outside of the doAs and interact with the client outside of this block, in the same way we could if using UGI? If so, maybe update the example to it, so these blocks are as apples-to-apples as they can get.


http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@206
PS3, Line 206: relogin
nit: "re-login" for consistency



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 21:49:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@201
PS1, Line 201: conntect
> nit: connect
I like the idea of showing the construction of the kudu client like below.


http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@211
PS1, Line 211: my-ap
> nit: not related to this patch, but how about naming this 'app-user' or som
+1



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 04:01:47 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16940

to look at the new patch set (#4).

Change subject: [java] Add JAAS example to client API doc
......................................................................

[java] Add JAAS example to client API doc

The AsyncKuduClient class documents how to connect to a secure Kudu
cluster using ticket cache or using a keytab when the application is
using hadoop-common libraries. We can't always rely on
UserGroupInformation though, so this commit expands the keytab-based
login case by a JAAS config example.

It also fixes a formatting issue with the "@Override" in the code, as it
wasn't properly rendered.

Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 36 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/16940/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16940

to look at the new patch set (#3).

Change subject: [java] Add JAAS example to client API doc
......................................................................

[java] Add JAAS example to client API doc

The AsyncKuduClient class documents how to connect to a secure Kudu
cluster using ticket cache or using a keytab when the application is
using hadoop-common libraries. We can't always rely on
UserGroupInformation though, so this commit expands the keytab-based
login case by a JAAS config example.

It also fixes a formatting issue with the "@Override" in the code, as it
wasn't properly rendered.

Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 37 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/16940/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@195
PS3, Line 195: n,
> nit: add a comma
Done


http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@201
PS3, Line 201:  *                             (P
> I'm curious, can we declare 'client' outside of the doAs and interact with 
Yea that makes sense, thanks for the tip. I just tried it that way and it works too. Updated.


http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@206
PS3, Line 206: 
> nit: "re-login" for consistency
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 22:02:10 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 4: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 Jan 2021 08:47:13 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 Jan 2021 08:59:22 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@183
PS3, Line 183:  *   com.sun.security.auth.module.Krb5LoginModule required
             :  *   useKeyTab = true
             :  *   keyTab = "/path/to/app.keytab"
> Cool, thank you for clarifying.  I asked because this document provides a s
Right, this is basically the same thing though, this could be written in a single line. Your example lists multiple login modules, one in each line. My example lists one login module with each parameter in a separate line.



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 20:49:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16940

to look at the new patch set (#2).

Change subject: [java] Add JAAS example to client API doc
......................................................................

[java] Add JAAS example to client API doc

The AsyncKuduClient class documents how to connect to a secure Kudu
cluster using ticket cache or using a keytab when the application is
using hadoop-common libraries. We can't always rely on
UserGroupInformation though, so this commit expands the keytab-based
login case by a JAAS config example.

It also fixes a formatting issue with the "@Override" in the code, as it
wasn't properly rendered.

Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 36 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/16940/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3: Code-Review+2

This looks good to me.

But I'd suggest to get approval from Andrew as well before merging.

Thanks!


-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 21:18:30 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@183
PS3, Line 183:  *   com.sun.security.auth.module.Krb5LoginModule required
             :  *   useKeyTab = true
             :  *   keyTab = "/path/to/app.keytab"
nit: should these lines end with semi-colon (;) as well or this is OK syntax even without those trailing semi-columns?



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 18:44:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@201
PS1, Line 201: uClient 
> I like the idea of showing the construction of the kudu client like below.
Done


http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@211
PS1, Line 211: code 
> +1
Changed it to appuser, this way we can keep the line length within 80 which seems to be the convention here.


http://gerrit.cloudera.org:8080/#/c/16940/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@220
PS1, Line 220: 
             :  *   );
> Is anything equivalent done for JAAS? Do JAAS users need to worry about doi
There's a note above about having to relogin manually. Added that it needs to be done periodically to make it clearer.



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 09:03:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 20:54:57 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................

[java] Add JAAS example to client API doc

The AsyncKuduClient class documents how to connect to a secure Kudu
cluster using ticket cache or using a keytab when the application is
using hadoop-common libraries. We can't always rely on
UserGroupInformation though, so this commit expands the keytab-based
login case by a JAAS config example.

It also fixes a formatting issue with the "@Override" in the code, as it
wasn't properly rendered.

Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Reviewed-on: http://gerrit.cloudera.org:8080/16940
Tested-by: Attila Bukor <ab...@apache.org>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 36 insertions(+), 6 deletions(-)

Approvals:
  Attila Bukor: Verified
  Andrew Wong: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Add JAAS example to client API doc

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16940 )

Change subject: [java] Add JAAS example to client API doc
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/16940/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@183
PS3, Line 183:  *   com.sun.security.auth.module.Krb5LoginModule required
             :  *   useKeyTab = true
             :  *   keyTab = "/path/to/app.keytab"
> nit: should these lines end with semi-colon (;) as well or this is OK synta
Yea this is the official syntax. Only the last line requires a semi-colon.



-- 
To view, visit http://gerrit.cloudera.org:8080/16940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7db5bf67727f7318272de7ea2156a33255a2dc8b
Gerrit-Change-Number: 16940
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 12 Jan 2021 19:07:14 +0000
Gerrit-HasComments: Yes