[Impala-ASF-CR] IMPALA-4277: allow overriding of Hive/Hadoop versions/locations

[Charlie Helin (Code Review)]

Charlie Helin 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?


Line 146: export HADOOP_LZO="${HADOOP_LZO-$IMPALA_HOME/../hadoop-lzo}"
How is this assigned?


Line 289: export KUDU_JAVA_VERSION=1.0.0-SNAPSHOT
This shouldn't be configurable too?


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 me 
that HADOOP_HOME also should be configurable, otherwise we would assume that 
the components are laid out in particular fashion.


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


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


-- 
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 
Gerrit-Reviewer: Charlie Helin 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4277: allow overriding of Hive/Hadoop versions/locations

[Charlie Helin (Code Review)]

Charlie Helin has posted comments on this change.

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


Patch Set 1: -Code-Review

> Did a grep for HADOOP_HOME, and this might need to be changed too:
 > https://github.com/apache/incubator-impala/blob/75a857c0ceb6a58691ff45f0df8d88586edc8acd/buildall.sh#L357

Yes here it seems like ${HADOOP_HOME}\lib should categorically be substituted 
by ${HADOOP_LIB_DIR}. But regarding this line it also looks like we need to 
parameterize HADOOP LZO?

-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Charlie Helin 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR](hadoop-next) IMPALA-4277: bump Hadoop component versions except for Hadoop itself

[Charlie Helin (Code Review)]

Charlie Helin has posted comments on this change.

Change subject: IMPALA-4277: bump Hadoop component versions except for Hadoop 
itself
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4698/1//COMMIT_MSG
Commit Message:

PS1, Line 16: This builds if I manually put hive_metastore.thrift in the right 
place
> We don't have the special component tarballs for CDH6.x, so I just did this
Ohh I see that looks better!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0be6bbe76d929ceaeb2fa9ac4ebba1820c4dab7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: hadoop-next
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Charlie Helin 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4277: allow overriding of Hive/Hadoop versions/locations

[Charlie Helin (Code Review)]

Charlie Helin has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

Line 305: export 
IMPALA_LLAMA_MINIKDC_VERSION=${IMPALA_LLAMA_MINIKDC_VERSION:-1.0.0}
Should this not be removed, in C6 we won't have llama?


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Charlie Helin 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR](hadoop-next) IMPALA-4277: bump Hadoop component versions except for Hadoop itself

[Charlie Helin (Code Review)]

Charlie Helin has posted comments on this change.

Change subject: IMPALA-4277: bump Hadoop component versions except for Hadoop 
itself
..


Patch Set 1:

(1 comment)

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

Line 299
Should this not also be bumped?

Also I would make these configurable using something like:

export FOO_VERSION=${FOO_VERSION:-2.0.0-...}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0be6bbe76d929ceaeb2fa9ac4ebba1820c4dab7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: hadoop-next
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Charlie Helin 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes