Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12846 )
Change subject: IMPALA-8345 : Add option to set up minicluster to use Hive 3 ...................................................................... Patch Set 9: (5 comments) http://gerrit.cloudera.org:8080/#/c/12846/9/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/12846/9/bin/bootstrap_toolchain.py@433 PS9, Line 433: def download_cdp_hive(toolchain_root): We don't have to do it now, but at some point, we should refactor this function to be more generic, like downloading Ranger, Hive, etc. http://gerrit.cloudera.org:8080/#/c/12846/9/bin/create-test-configuration.sh File bin/create-test-configuration.sh: http://gerrit.cloudera.org:8080/#/c/12846/9/bin/create-test-configuration.sh@132 PS9, Line 132: # Certain configurations (like SentrySyncHMSNotificationsPostListener) do not work : # with HMS 3.1.0. Use a cdp specific configuration template : generate_config postgresql-hive-site.xml.cdp.template hive-site.xml will this cause Sentry tests to fail when USE_CDP_HIVE=true? http://gerrit.cloudera.org:8080/#/c/12846/9/bin/create-test-configuration.sh@149 PS9, Line 149: 1>${IMPALA_CLUSTER_LOGS_DIR}/schematool.log 2>&1 it may be better to use tee 2>&1 | tee ${IMPALA_CLUSTER_LOGS_DIR}/schematool.log http://gerrit.cloudera.org:8080/#/c/12846/9/fe/src/test/resources/postgresql-hive-site.xml.cdp.template File fe/src/test/resources/postgresql-hive-site.xml.cdp.template: http://gerrit.cloudera.org:8080/#/c/12846/9/fe/src/test/resources/postgresql-hive-site.xml.cdp.template@27 PS9, Line 27: <property> nit: formatting is off in this file, a lot of mixed 1 space vs 2 spaces. http://gerrit.cloudera.org:8080/#/c/12846/9/testdata/bin/run-hive-server.sh File testdata/bin/run-hive-server.sh: http://gerrit.cloudera.org:8080/#/c/12846/9/testdata/bin/run-hive-server.sh@72 PS9, Line 72: if [[ $USE_CDP_HIVE && -n "$SENTRY_HOME" ]]; then : for f in ${SENTRY_HOME}/lib/sentry-binding-hive*.jar; do : FILE_NAME=$(basename $f) : # exclude all the hive jars from being included in the classpath since Sentry : # depends on Hive 2.1.1 : if [[ ! $FILE_NAME == hive* ]]; then : export HADOOP_CLASSPATH=${HADOOP_CLASSPATH}:${f} : fi : done : fi nit: use 2 spaces -- To view, visit http://gerrit.cloudera.org:8080/12846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfed856c1f5429ed45fd3d9cb08a5d1bb96a9605 Gerrit-Change-Number: 12846 Gerrit-PatchSet: 9 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Wed, 27 Mar 2019 01:50:55 +0000 Gerrit-HasComments: Yes
