Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20731 )

Change subject: IMPALA-12502: Support Impala to Impala federation
......................................................................


Patch Set 18:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/20731/17/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/20731/17/bin/impala-config.sh@192
PS17, Line 192: unset IMPALA_SNAPPY_URL
> fixed
Done


http://gerrit.cloudera.org:8080/#/c/20731/18/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
File fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java:

http://gerrit.cloudera.org:8080/#/c/20731/18/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java@839
PS18, Line 839:       LOG.info("Authentication failed with error: " + 
e.getMessage());
assertTrue can take a 2nd argument to print an error message that could provide 
this. I'd rather do that than use logging.


http://gerrit.cloudera.org:8080/#/c/20731/18/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
File 
java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java:

http://gerrit.cloudera.org:8080/#/c/20731/18/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java@288
PS18, Line 288:     if (jdbcAuth != null && !jdbcAuth.isEmpty()) {
nit: we have a helper for this, Strings.isNullOrEmpty


http://gerrit.cloudera.org:8080/#/c/20731/17/testdata/bin/download-impala-jdbc-driver.sh
File testdata/bin/download-impala-jdbc-driver.sh:

http://gerrit.cloudera.org:8080/#/c/20731/17/testdata/bin/download-impala-jdbc-driver.sh@32
PS17, Line 32: DRIVER_JAR_VERSION=${IMPALA_SIMBA_JDBC_DRIVER_VERSION%-*}
> fixed by extracting 42 from IMPALA_SIMBA_JDBC_DRIVER_VERSION
Done


http://gerrit.cloudera.org:8080/#/c/20731/17/testdata/bin/download-impala-jdbc-driver.sh@36
PS17, Line 36: if [ ! -z "$found" ]; then
> fixed
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79ad3273932b658cb85c9c17cc834fa1b5fbd64f
Gerrit-Change-Number: 20731
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <gsi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 23:07:11 +0000
Gerrit-HasComments: Yes

Reply via email to