impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3306: Add command-line flags to set S3 access configurations
Date Wed, 27 Apr 2016 21:42:54 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-3306: Add command-line flags to set S3 access configurations
......................................................................


Patch Set 5: Code-Review+1

(5 comments)

http://gerrit.cloudera.org:8080/#/c/2850/5//COMMIT_MSG
Commit Message:

Line 16: world readable
'potentially insecure'


http://gerrit.cloudera.org:8080/#/c/2850/5/be/src/common/init.cc
File be/src/common/init.cc:

Line 210: ABORT_IF_ERROR
Should this be ABORT? We return a bad status if the cache can't execute the command-line scripts,
which isn't an exceptional error (usually!).


http://gerrit.cloudera.org:8080/#/c/2850/5/be/src/runtime/hdfs-fs-cache.h
File be/src/runtime/hdfs-fs-cache.h:

Line 46:  if either of the provided commands fail.
Doesn't make sense w/o knowing what the 'commands' are. Instead, just say "returns an error
if initialisation was unsuccessful" (or maybe better: just remove this line).


http://gerrit.cloudera.org:8080/#/c/2850/5/be/src/util/os-util.h
File be/src/util/os-util.h:

Line 46: trim_right(*msg)
nit: clearer to say "all trailing whitespace is removed from the output".


http://gerrit.cloudera.org:8080/#/c/2850/5/tests/custom_cluster/test_s3a_access.py
File tests/custom_cluster/test_s3a_access.py:

Line 66:     in a custom cluster test.'''
Reference the JIRA you filed (e.g. "... test. See IMPALA-XXYY".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ba103bcb399861683066fd00219d30c180db043
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message