Todd Lipcon has posted comments on this change.

Change subject: Get and use Kudu from the toolchain by default
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/1985/3/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

Line 214:   ("KUDU_NOT_SUPPORTED_ON_OS", 69, "Kudu is not supported on this 
operating system.")
> I don't know if that has been decided yet. Alex and I just talked about thi
Yea, though we reserve the right to break it if necessary (i.e the 5.8 impala 
release may cease to work against Kudu 0.9 or 1.0 or whatever) depending on 
timing.


http://gerrit.cloudera.org:8080/#/c/1985/3/testdata/cluster/node_templates/cdh5/etc/init.d/kudu-common
File testdata/cluster/node_templates/cdh5/etc/init.d/kudu-common:

Line 1: if [[ -n $KUDU_BUILD_DIR ]]; then
> The quotes are there now.
they recommend that every file have the license to make it clear to auditors 
that it's original. If a file doesn't have the license, people have to wonder 
"hrmm, did they copy-paste this from somewhere"?

I'm OK leaving it off for now and bulk adding later if this is the norm.


Line 5:   if $USE_KUDU_DEBUG_BUILD; then
> Added a comment at the top
the new comment says "or USE_KUDU_DEBUG_BUILD" but should be "and", right? bash 
has the weird behavior that an unset variable here actually ends up evaluating 
to true:

todd@todd-ThinkPad-T540p:/tmp$ echo $USE_KUDU_DEBUG_BUILD
todd@todd-ThinkPad-T540p:/tmp$ cat test.sh
if $USE_KUDU_DEBUG_BUILD; then
  echo use debug
else
  echo use release
fi
todd@todd-ThinkPad-T540p:/tmp$ bash test.sh
use debug


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3db88cbd27f2ea2394f011bc8d1face37411ed58
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to