Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )
Change subject: IMPALA-2567: Enable KRPC by default ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/9461/3/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/9461/3/bin/start-impala-cluster.py@57 PS3, Line 57: Thrift thrift is always enabled. How about "Enable Thrift DataStream service ..." Also, flag effect would probably be clearer if named --disable_krpc http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py File tests/common/test_skip.py: http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py@35 PS3, Line 35: assert pytest.config.option.test_thrift : : @SkipIf.not_thrift : def test_skip_if_not_thrift(self): : assert not pytest.config.option.test_thrift these look backwards. don't you have to move the 'not'? http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py File tests/conftest.py: http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py@120 PS3, Line 120: Thrift enabled Thrift DataStream service enabled. Or "KRPC disabled". Whichever is consistent with what you decide in the other place. -- To view, visit http://gerrit.cloudera.org:8080/9461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0 Gerrit-Change-Number: 9461 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Fri, 02 Mar 2018 19:08:12 +0000 Gerrit-HasComments: Yes
