Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20682 )
Change subject: IMPALA-9086: Show Hive configurations in /hadoop-varz page It is a two part improvement. 1. updating /hadoop-varz static class implementation from: org.apache.hadoop.conf.Configuration to : org.apache.hadoop.hive.conf.HiveConf (inherrits Configurat ...................................................................... Patch Set 1: (8 comments) Overall looks good to me. Just have some suggestions on comments. http://gerrit.cloudera.org:8080/#/c/20682/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20682/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-9086: Show Hive configurations in /hadoop-varz page Add a blank line after the title http://gerrit.cloudera.org:8080/#/c/20682/1//COMMIT_MSG@13 PS1, Line 13: nit: fix the indention http://gerrit.cloudera.org:8080/#/c/20682/1//COMMIT_MSG@15 PS1, Line 15: about about nit: two "about" http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniCatalog.java@126 PS1, Line 126: IMPALA-9086: We don't need to mention the JIRA id for each changes, unless there are lots of details in the JIRA. http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@629 PS1, Line 629: getHadoopConfigAsHtml nit: move this commend on HIVE_CONF and mention org.apache.hadoop.hive.conf.HiveConf inherrits org.apache.hadoop.conf.Configuration. http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@636 PS1, Line 636: If asText is true, output in raw text. Otherwise, output in html. please help to remove this stale comment. http://gerrit.cloudera.org:8080/#/c/20682/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@638 PS1, Line 638: * IMPALA-9086 : /hadoop-varz http request has been updated to include Hive configurations We don't need comments on the current change. We can mention this method Returns all loaded Hadoop configuration parameters, including Hive configurations. http://gerrit.cloudera.org:8080/#/c/20682/1/tests/custom_cluster/test_web_pages.py File tests/custom_cluster/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/20682/1/tests/custom_cluster/test_web_pages.py@194 PS1, Line 194: # IMPALA-9086: updated test to check for /hadoop-varz in catalog webUI nit: don't need this comment. -- To view, visit http://gerrit.cloudera.org:8080/20682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5af0eb68e71afeed64660d4d40584208ea503217 Gerrit-Change-Number: 20682 Gerrit-PatchSet: 1 Gerrit-Owner: Saurabh Katiyal <saurabhkati...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 09 Nov 2023 02:15:18 +0000 Gerrit-HasComments: Yes
