Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22705 )
Change subject: IMPALA-13919: Allow running minicluster with Java 17 ...................................................................... Patch Set 12: (7 comments) http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh File bin/set-impala-java-tool-options.sh: http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh@20 PS11, Line 20: # Set JAVA_TOOL_OPTIONS needed by some Hadoop dependencies when running with JDK>=9 > In this comment, please tell us what to do once all components has been bui Good question, this depends on the dependencies we will have in the future. Some may still need this env var. http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh@63 PS11, Line 63: ADD_OPENS_OPTS+=" --add-opens=jdk.management/co > Might be useful to have initial value for IMPALA_JAVA_TOOL_OPTIONS in jenki Done http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java File fe/src/test/java/org/apache/impala/testutil/TestUtils.java: http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@492 PS11, Line 492: version = version.substring(2, 3); > Assert that version.equals("8") here? added precondition in line 499 http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@495 PS11, Line 495: if > nit: missing whitespace. Done http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py File fe/src/test/resources/hive-site.xml.py: http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py@23 PS11, Line 23: HIVE_MAJOR_VERSION = int(os.environ['IMPALA_HIVE_VERSION'][0]) : KERBERIZE = os.environ.get('IMPALA_KERBERIZE') == 'true' : VARIANT = os.environ.get('HIVE_VARIANT') : JDK_VERSION = os.environ.get('IMPALA_JDK_VERSION_NUM') > Change these constants to upper case as well? Done http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py@28 PS11, Line 28: IMPALA_JAVA_ > How about simply name this IMPALA_JAVA_TOOL_OPTIONS? Done http://gerrit.cloudera.org:8080/#/c/22705/11/tests/custom_cluster/test_catalog_hms_failures.py File tests/custom_cluster/test_catalog_hms_failures.py: http://gerrit.cloudera.org:8080/#/c/22705/11/tests/custom_cluster/test_catalog_hms_failures.py@30 PS11, Line 30: : NUM_SUBSCRIBERS = DEFAULT_CLUSTER_SIZE + 1 : : : @SkipIf.is_test_jdk > flake8 should ask for 2 new lines here. Wonder why it silent this time. Done -- To view, visit http://gerrit.cloudera.org:8080/22705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If58b64a21d14a4a55b12dfe9ea0b9c3d5fe9c9cf Gerrit-Change-Number: 22705 Gerrit-PatchSet: 12 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Tue, 01 Apr 2025 13:21:29 +0000 Gerrit-HasComments: Yes
