stak...@cloudera.com has posted comments on this change.

Change subject: IMPALA-3980: qgen: re-enable Hive as a target database
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4011/2//COMMIT_MSG
Commit Message:

PS2, Line 7: IMPALA-3980: qgen: re-enable Hive as a target database
> I prefer commit messages that, after the bug, speak of the code section cha
Done


PS2, Line 19: * Hive integration tested locally by invoking the data generator 
via the command:
            : 
            : ./data-generator.py \
            : --db-name=functional \
            : --use-hive \
            : --min-row-count=50 \
> While it makes the commit message have more lines, it is more readable to w
Done


http://gerrit.cloudera.org:8080/#/c/4011/2/tests/comparison/cli_options.py
File tests/comparison/cli_options.py:

PS2, Line 150:   group.add_argument('--hive-host', default=DEFAULT_HIVE_HOST,
             :       help="The name of the host running the HS2")
             :   group.add_argument("--hive-port", default=DEFAULT_HIVE_PORT, 
type=int,
             :       help="The port of HiveServer2")
> Why not derive these defaults from symbols imported from cluster.py? (The c
Done


http://gerrit.cloudera.org:8080/#/c/4011/2/tests/comparison/cluster.py
File tests/comparison/cluster.py:

PS2, Line 166: class MiniCluster(Cluster):
> This changes the MiniCluster constructor interface for all callers. It also
Done


PS2, Line 178:   def _init_local_hadoop_conf_dir(self):
> Thanks for doing the cleanups like this one.
No problem! It makes the code a lot easier to run locally.


http://gerrit.cloudera.org:8080/#/c/4011/2/tests/comparison/data_generator.py
File tests/comparison/data_generator.py:

PS2, Line 101:   def populate_db(self, table_count, postgresql_conn=None, 
is_hive=False)
> We don't use variable names of the format "isHive" typically, so can you ch
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stak...@cloudera.com
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: stak...@cloudera.com
Gerrit-HasComments: Yes

Reply via email to