Zach Amsden has posted comments on this change.

Change subject: IMPALA-5764: Allow overriding packaged components
......................................................................


Patch Set 2:

(4 comments)

A couple of minor nits.  Also, let me make sure I didn't accidentally introduce 
typos during edits and that the script and data load still work.

http://gerrit.cloudera.org:8080/#/c/7581/2/README.md
File README.md:

Line 69: | PACKAGED_COMPONENTS_NAME | "cdh" | Identifier used to uniquify paths 
for potentially incompatible component builds. |
This doesn't currently match the "cdh5" label used in the shell script.


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

Line 180: export 
DOWNLOAD_PACKAGED_COMPONENTS=${DOWNLOAD_PACKAGED_COMPONENTS-"$NO_THIRDPARTY"}
Technically this "breaks" the C6 build, but because the cdh6.x version of 
Impala is pushed right now from CDH5 with a thirdparty directory, this is still 
working.  Will need to update the cauldron build to fix this.


Line 361:   export 
PACKAGED_COMPONENTS_HOME="$IMPALA_TOOLCHAIN/$PACKAGED_COMPONENTS_NAME"
This changes the download directory for the CDH pieces of the toolchain; I 
presume that is okay to do, might result in duplicate copies but nothing should 
break because of this.


Line 382: export 
MINI_DFS_BASE_DATA_DIR="$IMPALA_HOME/${PACKAGED_COMPONENTS_NAME}-hdfs-data"
This forces a data reload - unless there is any objection, I'm not going to try 
to change that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I95c2662e6f62adc924cc5de7a371202126046545
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zach Amsden <[email protected]>
Gerrit-HasComments: Yes

Reply via email to