David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8894 )

Change subject: IMPALA-6372: Go parallel for Hive dataload
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8894/7/bin/load-data.py
File bin/load-data.py:

http://gerrit.cloudera.org:8080/#/c/8894/7/bin/load-data.py@292
PS7, Line 292:     execution_function = exec_hive_query_from_file_beeline
> "Switched most print statements to python logging. Most are at the INFO 
> level."

This is actually a welcome change. It looks like there are only a few more 
instances of print statements left in the file. Any chance you'd be willing to 
just zap them all?


http://gerrit.cloudera.org:8080/#/c/8894/9/bin/load-data.py
File bin/load-data.py:

http://gerrit.cloudera.org:8080/#/c/8894/9/bin/load-data.py@139
PS9, Line 139:     if error_msg: error_msg = "%s: %s" % (error_msg, str(e))
I may be missing something obvious here, but it looks like there maybe be a 
chance for str(e) to be swallowed here.

If error_msg has a value, as in the case of...

  exec_cmd(hbase_cmd, error_msg='Error executing hbase create commands')

then error_msg gets concatenated with str(e) and the result is logged in the 
finally-clause.

But if error_msg is None, it looks like we just eat the exception and then 
don't log anything.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34b71e6df3c8f23a5a31451280e35f4dc015a2fd
Gerrit-Change-Number: 8894
Gerrit-PatchSet: 9
Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:34:55 +0000
Gerrit-HasComments: Yes

Reply via email to