Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-3980: qgen: re-enable Hive as a target database ......................................................................
Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/4011/7/tests/comparison/cluster.py File tests/comparison/cluster.py: Line 100: if default is None: I think it's a little confusing to check if default is None here. How about: result = self._hadoop_configs.get(key, default) if result is None: raise ... return result Line 185: other_conf_dir = os.environ["HIVE_CONF_DIR"] I'm not sure that modifying the minicluster here is the right thing to do. It represents the minicluster environment in the Impala repository. How are you running, the Hive tests? Do you use this environment or you run against a cluster? http://gerrit.cloudera.org:8080/#/c/4011/7/tests/comparison/data_generator.py File tests/comparison/data_generator.py: Line 92: self.db_name = None How about adding a variable here called, for example, db_engine that can be either set to Hive or Impala? Then we won't have to worry about passing is_hive to populate_db every time. Line 153: cursor.compute_stats() We don't need to compute stats if we are using hive? -- 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: 7 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
