impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3223: Supports download of CDH components from S3.
Date Fri, 10 Jun 2016 19:25:19 GMT
Michael Ho has posted comments on this change.

Change subject: IMPALA-3223: Supports download of CDH components from S3.
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3333/2/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

Line 15: # Bootstrapping the native toolchain with prebuilt binaries
> This comment and the script name is getting increasingly inaccurate now tha
Comments updated. Keep the name of the script the same for now.


Line 288: def download_cdh_components(toolchain_root, cdh_components):
> I have SKIP_TOOLCHAIN_BOOTSTRAP set to true in my environment, so I guess i
Updated the script to not download the package at all if the package directory exists. Will
not use the md5sum files anymore.


Line 342: 
> Extra line in comment.
Done


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

Line 49: : ${SKIP_TOOLCHAIN_BOOTSTRAP=false}
> Maybe a one-liner comment?
Done


http://gerrit.cloudera.org:8080/#/c/3333/2/buildall.sh
File buildall.sh:

Line 253:   if [ "$?" == "0" ]; then
> More concisely:
Done. Don't need the if-statement at all as you said below.


Line 255:   else
> Actually, set -e already fails the script if the bootstrap_toolchain.py com
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16fa79db0005554cc0a116e74775647ba99f8dda
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message