Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17660 )
Change subject: IMPALA-10778: Allow impala-shell to connect directly to HS2 ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@192 PS7, Line 192: self.ldap_password = 'password' > can you add a comment about the reason for using a dummy password? Done http://gerrit.cloudera.org:8080/#/c/17660/7/shell/impala_shell.py@574 PS7, Line 574: return StrictHS2Client(self.impalad, self.fetch_size, self.kerberos_host_fqdn, : self.use_kerberos, self.kerberos_service_name, self.use_ssl, : self.ca_cert, self.user, self.ldap_password, True, : self.client_connect_timeout_ms, self.verbose, : use_http_base_transport=False, http_path=self.http_path, : http_cookie_names=None) > optional: I think that using a member like is_strict_mode could be a better I don't know... I hear what you're saying. But if someone is adding a parameter, I would hope that they notice the other flavor here and realize it is needed? All the calls to the constructor are right here. I really don't like adding the "if" clause, since I think it dirties up the code. http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@283 PS7, Line 283: "Only useful if connecting straight to hs2 instead of Impala.") > Can you mention that 10001 is the default port in this case? (btw, what is Done http://gerrit.cloudera.org:8080/#/c/17660/7/shell/option_parser.py@284 PS7, Line 284: test_mode > I would prefer a more exact name like --use_ldap_test_password. We use impa Done -- To view, visit http://gerrit.cloudera.org:8080/17660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I674a45640a4a7b3c9a577830dbc7b16a89865a9e Gerrit-Change-Number: 17660 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 09 Aug 2021 21:21:50 +0000 Gerrit-HasComments: Yes
