You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2016/10/13 22:22:06 UTC

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

Tim Armstrong has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4720

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

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

This is to help with IMPALA-4277 to make it easier to build against
Hadoop/Hive distributions where the directory layout doesn't exactly
match our current CDH dependencies, or where we may want to
temporarily override a version without making a source change.

Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
---
M bin/impala-config.sh
M cmake_modules/FindHDFS.cmake
M common/thrift/CMakeLists.txt
3 files changed, 23 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/4720/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#3).

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

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

This is to help with IMPALA-4277 to make it easier to build against
Hadoop/Hive distributions where the directory layout doesn't exactly
match our current CDH dependencies, or where we may want to
temporarily override a version without making a source change.

Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
---
M bin/impala-config.sh
M buildall.sh
M cmake_modules/FindHDFS.cmake
M common/thrift/CMakeLists.txt
4 files changed, 27 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/4720/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

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


Patch Set 1:

We will probably want to make the component versions configurable too so it's possible to build against Apache Hadoop etc.

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

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


Patch Set 1:

I updated the HADOOP_HOME/lib reference

I think HADOOP_LZO shouldn't need updating at this stage since it's a test-only dependency, and for IMPALA_LZO it should always be pointing at the IMPALA_LZO source repo, so there isn't really any variation in layout to configure.

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

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


Patch Set 1:

(2 comments)

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}
> This is actually separate from Llama - a test utility we stole from an old 
I'm also going to post a separate patch to remove Llama (needed to do some testing there to make sure it didn't break anything).


PS1, Line 341: $HADOOP_HOME/bin
> If HADOOP_INCLUDE_DIR and HADOOP_LIB_DIR point to a different path than HAD
I think if they were set to incompatible versions it might cause a problem since we'd be building against the wrong version of Hadoop (e.g Hadoop 2 API versus Hadoop 3 API). I think it's reasonable to assume that it won't be set up to point to incompatible versions.


-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Charlie Helin (Code Review)" <ge...@cloudera.org>.
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

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


Patch Set 2:

(4 comments)

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

PS2, Line 333: exanded
> "expanded"
Done


PS2, Line 335: ${HADOOP_HOME}/share/hadoop/tools/lib/*
> Just checking, but does this need updating too?
I don't think right now - we don't need this to build Impala.


PS2, Line 404: ${HADOOP_HOME}/lib/native/
> There are more here.
Done


PS2, Line 424: ${HADOOP_HOME}/lib/native
> And here.
Done. I suspect this line isn't necessary but we can leave that to a separate change.


-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil 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:

PS1, Line 341: $HADOOP_HOME/bin
If HADOOP_INCLUDE_DIR and HADOOP_LIB_DIR point to a different path than HADOOP_HOME/(include)|(lib)/ , wouldn't this cause an issue?


-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

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


Patch Set 3: Code-Review+2

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Charlie Helin (Code Review)" <ge...@cloudera.org>.
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2).

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

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

This is to help with IMPALA-4277 to make it easier to build against
Hadoop/Hive distributions where the directory layout doesn't exactly
match our current CDH dependencies, or where we may want to
temporarily override a version without making a source change.

Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
---
M bin/impala-config.sh
M buildall.sh
M cmake_modules/FindHDFS.cmake
M common/thrift/CMakeLists.txt
4 files changed, 24 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/4720/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

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


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

This is to help with IMPALA-4277 to make it easier to build against
Hadoop/Hive distributions where the directory layout doesn't exactly
match our current CDH dependencies, or where we may want to
temporarily override a version without making a source change.

Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Reviewed-on: http://gerrit.cloudera.org:8080/4720
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Internal Jenkins
---
M bin/impala-config.sh
M buildall.sh
M cmake_modules/FindHDFS.cmake
M common/thrift/CMakeLists.txt
4 files changed, 27 insertions(+), 21 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Sailesh Mukil: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7da10e38f9c4309f2d193dc25f14a6ea308c9639
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong 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?
This is actually separate from Llama - a test utility we stole from an old version of the Llama codebase and have kept around. Not quite sure what we're going to do about it but there's a JIRA IMPALA-4292.

It also doesn't affect the final build bits.


-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

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


Patch Set 3: Verified+1

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

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


Patch Set 1:

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

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

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


Patch Set 1:

Is this necessary for the master branch? Once we move permanently to C6, we wouldn't need these optional settings, am I right?

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Charlie Helin (Code Review)" <ge...@cloudera.org>.
Charlie Helin has posted comments on this change.

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


Patch Set 1: Code-Review+1

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

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


Patch Set 1:

This is for master, yet.

I think we will need to be able to point to the specific directories regardless to integrate with other build systems and let people build against arbitrary versions of dependencies.

The problem is that the thirdparty Hadoop/Hive/etc tarballs we download have a very specific directory layout that is not necessarily shared by other source/build trees we might want to build against. E.g. it would make sense to build against the Hive source directly, but in the Hive source repo hive_metastore.thrift is under ./metastore/if/hive_metastore.thrift instead of ./src/metastore/if/hive_metastore.thrift

-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

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

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

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


Patch Set 2:

(4 comments)

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

PS2, Line 333: exanded
"expanded"


PS2, Line 335: ${HADOOP_HOME}/share/hadoop/tools/lib/*
Just checking, but does this need updating too?


PS2, Line 404: ${HADOOP_HOME}/lib/native/
There are more here.


PS2, Line 424: ${HADOOP_HOME}/lib/native
And here.


-- 
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

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

Posted by "Charlie Helin (Code Review)" <ge...@cloudera.org>.
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 <ta...@cloudera.com>
Gerrit-Reviewer: Charlie Helin <ch...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes