Michael Brown has posted comments on this change. Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables ......................................................................
Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/4414/5/bin/start-impala-cluster.py File bin/start-impala-cluster.py: PS5, Line 73: parser.add_option("--kudu_masters", default="127.0.0.1", Could we import the default value from tests.common instead of hardcoding it? http://gerrit.cloudera.org:8080/#/c/4414/5/tests/common/kudu_test_suite.py File tests/common/kudu_test_suite.py: PS5, Line 38: @SkipIf.kudu_not_supported This is extremely risky due to bugs in our version of old pytest. See IMPALA-3614 for an example of how this is risky. I think you will have to work around this by skipping in setup_class. http://gerrit.cloudera.org:8080/#/c/4414/5/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: PS5, Line 1: # Copyright (c) 2016 Cloudera, Inc. All rights reserved. Remove Cloudera copyright and make sure the license text is correct otherwise. http://gerrit.cloudera.org:8080/#/c/4414/5/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS5, Line 220: Why remove this skip? PS5, Line 221: Is this removal safe? PS5, Line 221: self.expected_exceptions = 2 I can't see where this is used; delete? http://gerrit.cloudera.org:8080/#/c/4414/5/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: PS5, Line 87: print("Describe formatted output:") : pprint(table_desc) Use LOG.info() -- To view, visit http://gerrit.cloudera.org:8080/4414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b9d51b2720ab57649abdb7d5c710ea04ff50dc1 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-HasComments: Yes
