You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Michael Brown (Code Review)" <ge...@cloudera.org> on 2016/09/28 15:45:27 UTC

[Impala-ASF-CR] IMPALA-4207: test infra: move Hive options from connection to cluster options

Michael Brown has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4555

Change subject: IMPALA-4207: test infra: move Hive options from connection to cluster options
......................................................................

IMPALA-4207: test infra: move Hive options from connection to cluster options

Various test tools and frameworks, including the stress test, random
query generator, and nested types loader, share common modules. This
change

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

made changes to tests.comparison.cli_options, the shared command line
option module, and to tests.comparison.cluster, the shared module for
modeling various Impala clusters. Those changes were for the random
query generator, but didn't take into account the other shared entry
points. It was possible to call some of those entry points in such a way
as to produce an exception, because the Hive-related options are now
required for miniclusters, but the Hive-related options weren't always
being initialized in those entry points.

The simple fix is to say that, because Hive settings are now needed to
create Minicluster objects, make the Hive options initialized with
cluster options, not connection options. While I was making these
changes, I fixed flake8 problems in these methods only.

Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
---
M tests/comparison/cli_options.py
1 file changed, 31 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4555/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4555
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-4207: test infra: move Hive options from connection to cluster options

Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4207: test infra: move Hive options from connection to cluster options
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4555/1/tests/comparison/cli_options.py
File tests/comparison/cli_options.py:

PS1, Line 89: section
Maybe rename this to parser here and elsewhere to be consistent with add_minicluster_options?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4207: test infra: move Hive options from connection to cluster options

Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.

Sahil Takiar has posted comments on this change.

Change subject: IMPALA-4207: test infra: move Hive options from connection to cluster options
......................................................................


Patch Set 1: Code-Review+1

Just curious, what are the other modules that use cli_options? Are they inside the Impala repo?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4207: test infra: move Hive options from connection to cluster options

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4207: test infra: move Hive options from connection to cluster options
......................................................................


IMPALA-4207: test infra: move Hive options from connection to cluster options

Various test tools and frameworks, including the stress test, random
query generator, and nested types loader, share common modules. This
change

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

made changes to tests.comparison.cli_options, the shared command line
option module, and to tests.comparison.cluster, the shared module for
modeling various Impala clusters. Those changes were for the random
query generator, but didn't take into account the other shared entry
points. It was possible to call some of those entry points in such a way
as to produce an exception, because the Hive-related options are now
required for miniclusters, but the Hive-related options weren't always
being initialized in those entry points.

The simple fix is to say that, because Hive settings are now needed to
create Minicluster objects, make the Hive options initialized with
cluster options, not connection options. While I was making these
changes, I fixed all flake8 problems in this file.

Testing:

- qgen/minicluster unit tests (regression test)
- full private data load job, including load_nested.py (bug
  verification)
- data_generator.py run (regression test), long enough to verify
  connection to the minicluster, using both Hive and Impala
- discrepancy_searcher.py run (regression test), long enough verify
  connection to the minicluster, using both Hive and Impala
- concurrent_select.py (in typical mode using a CM host, this is a
  regression check; from the command line against the minicluster, this
  is a bug verification)

Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
Reviewed-on: http://gerrit.cloudera.org:8080/4555
Reviewed-by: Michael Brown <mi...@cloudera.com>
Reviewed-by: Ishaan Joshi <is...@cloudera.com>
Tested-by: Internal Jenkins
---
M tests/comparison/cli_options.py
M tests/comparison/cluster.py
2 files changed, 124 insertions(+), 78 deletions(-)

Approvals:
  Ishaan Joshi: Looks good to me, approved
  Michael Brown: Looks good to me, but someone else must approve
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2a2915e6db85ddb3d8e1bce8035eccd0c9324b4b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>