Ferenc Szabo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11334 )

Change subject: KUDU-2012 Kudu Flume sink auth support
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
File 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java@217
PS3, Line 217: 
> nit: indent at least 4 spaces on a line continuation
Checkstyle did not mention this indentation error.
You might need to fine-tune the checkstyle config.


http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java
File 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTestUtil.java@54
PS3, Line 54:     context.put(KERBEROS_KEYTAB, clusterRoot + 
"/krb5kdc/test-user.keytab");
            :     context.put(KERBEROS_PRINCIPAL, "test-u...@krbtest.com");
> Are there some kinds of prerequisites we should document in the javadoc for
This user is created by the mini cluster. I have only added the creation of the 
keytab file.


http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java
File 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java:

http://gerrit.cloudera.org:8080/#/c/11334/3/java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/SecureKuduSinkTest.java@57
PS3, Line 57:     System.clearProperty(KUDU_TICKETCACHE_PROPERTY);
> Could this affect other Kudu client tests? Should we be restoring this when
The current config is to fork jvm for every test class, so this does not affect 
the other test. The mini cluster sets it in the begining of the tests as well. 
It would be restored by the minicluster even without jvm isolation in the 
@Before of the BaseKuduTest



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11b5f08802883afa178d346af48d3bcd15281917
Gerrit-Change-Number: 11334
Gerrit-PatchSet: 3
Gerrit-Owner: Ferenc Szabo <fsz...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Ferenc Szabo <fsz...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Comment-Date: Wed, 26 Sep 2018 10:14:37 +0000
Gerrit-HasComments: Yes

Reply via email to