Csaba Ringhofer 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 3: (9 comments) http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@9 PS3, Line 9: Impala-shell already uses HS2 protocol to connect to Impalad. nit: extra whitespace http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@10 PS3, Line 10: any server Can you add "for example Hive"? http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@11 PS3, Line 11: --strict I would prefer a clearer name, like strict_hs2 or no_hs2_extensions. Strict could mean so many things. http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@12 PS3, Line 12: Can you add more info about what this client is capable of in strict mode? Can it connect to Hive? Can it connect to Impala? Can it use authentication mechanisms? One issue that comes up is that we use a different cookie name in Impala than in Hive, so cookie auth won't work with Hive. http://gerrit.cloudera.org:8080/#/c/17660/3//COMMIT_MSG@13 PS3, Line 13: When the "--strict" option is turned on Wouldn't it be possible to detect this somehow at runtime? For example by calling "select version()" first. http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_client.py@1105 PS3, Line 1105: return "" Shouldn't we assert when calling unsupported functions? Will this be actually used? http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@191 PS3, Line 191: test_mode Do we use test mode for anything else than setting ldap_password? if not, then why not simply pass the password? http://gerrit.cloudera.org:8080/#/c/17660/3/shell/impala_shell.py@574 PS3, Line 574: 'steve', 'password Will this work on other machines? http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/17660/3/tests/shell/test_shell_commandline.py@142 PS3, Line 142: if vector.get_value('strict_protocol') and vector.get_value('protocol') == 'beeswax': : pytest.skip("Beeswax protocol not supported in strict mode.") you can add this logic as a skipIf annotation in https://github.com/apache/impala/blob/master/tests/common/skip.py -- 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: 3 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Thu, 15 Jul 2021 17:38:53 +0000 Gerrit-HasComments: Yes
