Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17732 )

Change subject: [java] KUDU-1921 Add ability to require authn/encryption to 
Java client
......................................................................


Patch Set 8: Code-Review+1

(2 comments)

Overall looks good to me, just the same naming nit as for the C++ client and a 
request to add one extra test scenario.

http://gerrit.cloudera.org:8080/#/c/17732/8/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/17732/8/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2735
PS8, Line 2735:     // Only connects to remote servers that support encryption, 
fails
              :     // otherwise. It can connect to insecure servers only 
locally.
              :     REQUIRED,
              :     // Only connects to any server, including on the loopback 
interface,
              :     // that support encryption, fails otherwise.
              :     REQUIRED_LOOPBACK,
nit: consider renaming these as well per suggestion at 
https://gerrit.cloudera.org/#/c/17731/8/src/kudu/client/client.h@238


http://gerrit.cloudera.org:8080/#/c/17732/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java:

http://gerrit.cloudera.org:8080/#/c/17732/8/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@574
PS8, Line 574: }
While you are working on this, do you mind adding one extra test scenario to 
verify that a client with encryption policy 
AsyncKuduClient.EncryptionPolicy.OPTIONAL (the default setting) is able to 
connect to a cluster running with "--rpc_encryption=disabled", 
"--rpc_authentication=disabled" flags?  I thought we had such a test scenario 
in TestSecurity or elsewhere, but I didn't find such a scenario anywhere for 
Java Kudu client, actually.

Thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic951b2090a4933eca70dc53b6f93cdcff5a74929
Gerrit-Change-Number: 17732
Gerrit-PatchSet: 8
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 09 Aug 2021 22:18:33 +0000
Gerrit-HasComments: Yes

Reply via email to