Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7241 )

Change subject: IMPALA-2782: Allow impala-shell to connect directly to impalad 
when configured with load balancer and kerberos.
......................................................................


Patch Set 2:

I had to look at the JIRA to understand what this is trying to accomplish. That 
suggests that the important change in impala_client.py (where you set up the 
SASL) needs explanation. You've changed what Thrift/SASL/Kerberos is expecting, 
and it's worth having a comment that explains the scenario.

That said, I think Hrishikesh Gadre's comment on the JIRA is also worth 
addressing. It feels weird that when load-balancers are configured, impalad 
always has this issue (that it impersonates the load balancer?). Is that 
condition relaxable? The change you have works here, but it would need to be 
repeated for JDBC drivers and for impyla and anything else.

As a nit, I think "self.lb" is a poor variable name; self.load_balancer is 
better. Even there, I'd worry that the client is going to get confused: the 
point of the load balancer is that it acts like an impalad, so I worry people 
are going to specify both their impalad (?) and their load-balancer, even in 
scenarios when they don't need to. These are quite user-visible flags.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4726226a7a3817421b133f74dd4f4cf8c52135f9
Gerrit-Change-Number: 7241
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran <vtt...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vtt...@cloudera.com>
Gerrit-Reviewer: a...@phdata.io
Gerrit-Comment-Date: Wed, 29 Nov 2017 22:00:50 +0000
Gerrit-HasComments: No

Reply via email to