Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15134 )
Change subject: IMPALA-9279: Update the Kudu version to include VARCHAR support ...................................................................... Patch Set 2: (2 comments) Thanks for working on this. This is looking pretty good. I'm thinking through the edge cases where we want to override some versions, so I may have a couple more comments. In the meantime, I'm going to run an upstream verify job on this review. http://gerrit.cloudera.org:8080/#/c/15134/2/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/15134/2/bin/impala-config.sh@719 PS2, Line 719: export IMPALA_TOOLCHAIN_KUDU_MAVEN_REPOSITORY="file://${IMPALA_TOOLCHAIN}" Since this is disabled, I think we can set it to an empty string. If that works, use it. http://gerrit.cloudera.org:8080/#/c/15134/2/bin/impala-config.sh@722 PS2, Line 722: export IMPALA_KUDU_VERSION="3ba5ec5d0" : export IMPALA_KUDU_JAVA_VERSION="1.12.0-SNAPSHOT" One use case that we want to support is for someone to be able to override the IMPALA_KUDU_VERSION and IMPALA_KUDU_JAVA_VERSION and build against a custom Kudu. One might export these variables in bin/impala-config-branch.sh. So, IMPALA_KUDU_VERSION and IMPALA_KUDU_JAVA_VERSION should respect a preset if it is set. i.e. IMPALA_KUDU_VERSION=${IMPALA_KUDU_VERSION-"3ba5ec5d0"} IMPALA_KUDU_JAVA_VERSION=${IMPALA_KUDU_JAVA_VERSION-"1.12.0-SNAPSHOT"} -- To view, visit http://gerrit.cloudera.org:8080/15134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iafe56342d43cb63e35c0bbb1b4a99327dda0a44a Gerrit-Change-Number: 15134 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Comment-Date: Thu, 30 Jan 2020 20:04:12 +0000 Gerrit-HasComments: Yes
