Lars Volker has posted comments on this change. ( )

Change subject: IMPALA-6508: add KRPC test flag

Patch Set 4:


Thanks for the review, please see my inline comments and PS5.
File bin/
PS4, Line 57: krpc
File tests/common/
PS4, Line 138: "--impalad_args=-use_krpc"
> Not quite following what this translates to ? For now, I have been using --
Yes, -use_krpc is a boolean and as such its presence enables it. This page has 
all four variations that are supported for a boolean flag:

 app_containing_foo --big_menu
 app_containing_foo --nobig_menu
 app_containing_foo --big_menu=true
 app_containing_foo --big_menu=false

I picked the single-dash variant because I've seen this convention elsewhere I 
think. Let me know if you prefer two dashes.
File tests/common/
PS4, Line 89: krpc
> nit: KRPC
File tests/common/
PS4, Line 24: class TestSkipIf(ImpalaTestSuite):
> Is this test gonna be part of the patch or is it for testing only ?
My idea was to keep it in this patch so that we actually have a test for the 
things I added. Once we add a proper (non-custom-cluster) test using the 
SkipIfs I plan to drop this test again.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Comment-Date: Tue, 13 Feb 2018 04:07:55 +0000
Gerrit-HasComments: Yes

Reply via email to