Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/9291 )
Change subject: IMPALA-6508: add KRPC test flag ...................................................................... Patch Set 5: (3 comments) I was trying to think about how this could be done without yet another "global" environment variable, and an unenforceable flag between pytest and the cluster, and more skip logic. Do you think it's possible to achieve the same effect by: 1. Creating CI that sets TEST_START_CLUSTER_ARGS appropriately for enabling KRPC 2. Ensuring $TEST_START_CLUSTER_ARGS is properly reconciled any time the cluster starts, even in custom cluster tests. 3. Running the 3 tests that are KRPC specific with --use-krpc always set, unconditionally. I have a feeling this would reduce some of the churn that's introduced in this patch and then would be gutted again later. What do you think? http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py File tests/common/custom_cluster_test_suite.py: http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137 PS5, Line 137: if pytest.config.option.test_krpc: : cmd.append("--impalad_args=-use_krpc") It seems like a bug that custom cluster tests don't prepend $TEST_START_CLUSTER_ARGS with other custom args. http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py File tests/common/test_skip.py: http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py@18 PS5, Line 18: import os Unused import. http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py File tests/custom_cluster/test_krpc_mem_usage.py: http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77 PS5, Line 77: @CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000") An alternative to skipping would be to just force --use-krpc always to on in these tests. -- To view, visit http://gerrit.cloudera.org:8080/9291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab Gerrit-Change-Number: 9291 Gerrit-PatchSet: 5 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Tue, 13 Feb 2018 17:05:45 +0000 Gerrit-HasComments: Yes
