Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17793 )
Change subject: IMPALA-10870: Add Apache Hive 3.1.2 to the minicluster ...................................................................... Patch Set 4: (6 comments) Thanks for contributing this! I see some codes that dealing with *_OVERRIDE env vars are removed. Please keep them since we have internal jobs still depending on them. http://gerrit.cloudera.org:8080/#/c/17793/4/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/17793/4/bin/bootstrap_toolchain.py@a468 PS4, Line 468: : : : : : : Please don't remove these codes. We have internal jobs still using them. http://gerrit.cloudera.org:8080/#/c/17793/4/bin/bootstrap_toolchain.py@329 PS4, Line 329: APACHE_ARCHIVE Can we use APACHE_COMPONENTS_HOME instead? The implementation of this class has lots of hive stuffs. In the future, we may add other components like Hadoop, which are unrelated to Hive. It'd be good to be more abstractive. http://gerrit.cloudera.org:8080/#/c/17793/4/bin/bootstrap_toolchain.py@336 PS4, Line 336: url_prefix_tmpl = "${apache_archive}/${name}/%s/" % (baseversion_tmpl) I think different components have different base urls. We need a real prefix url as the input parameter. http://gerrit.cloudera.org:8080/#/c/17793/4/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/17793/4/bin/impala-config.sh@190 PS4, Line 190: : ${APACHE_ARCHIVE:=https://archive.apache.org/dist} It'd be better to download from apache mirrors. An example URL is http://www.apache.org/dyn/closer.cgi/hive/hive-3.1.2/apache-hive-3.1.2-src.tar.gz?action=download Ref: https://infra.apache.org/release-download-pages.html#closer http://gerrit.cloudera.org:8080/#/c/17793/4/bin/impala-config.sh@272 PS4, Line 272: ${CDP_HIVE_VERSION} Let's keep the original code, i.e. ${HIVE_VERSION_OVERRIDE:-"$CDP_HIVE_VERSION"}, so bin/impala-config-branch.sh or bin/impala-config-local.sh can still override this. http://gerrit.cloudera.org:8080/#/c/17793/4/bin/impala-config.sh@275 PS4, Line 275: "2.3.0.$IMPALA_HIVE_VERSION" Same as above. Let's don't remove HIVE_STORAGE_API_VERSION_OVERRIDE. -- To view, visit http://gerrit.cloudera.org:8080/17793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1978909589ecacb15d32d874e97f050a85adf1f6 Gerrit-Change-Number: 17793 Gerrit-PatchSet: 4 Gerrit-Owner: Fucun Chu <[email protected]> Gerrit-Reviewer: Fucun Chu <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 24 Aug 2021 14:06:58 +0000 Gerrit-HasComments: Yes
