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