Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4277: allow overriding of Hive/Hadoop versions/locations
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4720/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 145: export CDH_MAJOR_VERSION=5
> Bump?
There was a separate cleanup effort to remove CDH references from Apache Impala 
so I think this will be going away soon.


Line 146: export HADOOP_LZO="${HADOOP_LZO-$IMPALA_HOME/../hadoop-lzo}"
> How is this assigned?
This either comes from the environment (e.g. build script or .bashrc), 
otherwise it assumes that the Impala/hadoop-lzo/Impala-lzo repos are all under 
the same directory.


Line 289: export KUDU_JAVA_VERSION=1.0.0-SNAPSHOT
> This shouldn't be configurable too?
I'm not quite sure what the right thing to do here is since the Kudu 
integration is in flux and we're getting Kudu build artifacts in a different 
way than other components, so I wanted to punt for now until that's stabilised.


Line 325: export 
HADOOP_HOME="$CDH_COMPONENTS_HOME/hadoop-${IMPALA_HADOOP_VERSION}/"
> It's a bit confusing with 2 concurrent and related reviews. But it seems to
There's not much point making this more configurable right now since we assume 
these CDH component tarballs are constructed in a special way our test cluster 
expects. I'd rather keep it non-configurable until we've validated that we can 
drop in different build artifacts just to avoid confusion.

I validated that we can build Impala (but not run tests) with these pointing to 
nothing-in-particular.


Line 345: export 
SENTRY_HOME="$CDH_COMPONENTS_HOME/sentry-${IMPALA_SENTRY_VERSION}"
> same as 325
See above.


Line 348: export HIVE_HOME="$CDH_COMPONENTS_HOME/hive-${IMPALA_HIVE_VERSION}/"
> same as 325
See above.


-- 
To view, visit http://gerrit.cloudera.org:8080/4720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <che...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to