Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/11540 )
Change subject: IMPALA-7555: Set socket timeout in impala-shell ...................................................................... Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@265 PS8, Line 265: print_to_stderr('Connected to %s:%s' % (self.impalad_host, self.impalad_port)) This message seems misleading, given that the client has its own notion of being connected (see is_connected() above). We can change it to "Opened TCP connection to %s:%s" and only print it when the log level is set to debug. If you want to tell both hangs apart, we should use two separate log messages. http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@266 PS8, Line 266: # Setting a timeout of None disables timeouts on socket operations "on sockets." http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py@816 PS6, Line 816: except ImportError: I think we should keep this, see my comment elsewhere http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1510 PS8, Line 1510: given set_variables.""" nit: indent http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1540 PS8, Line 1540: """Look for a namespace:var_name pattern in an option name. While you're here, "Looks", "Returns" http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py@737 PS6, Line 737: """ Where did this go? -- To view, visit http://gerrit.cloudera.org:8080/11540 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813 Gerrit-Change-Number: 11540 Gerrit-PatchSet: 8 Gerrit-Owner: Anuj Phadke <[email protected]> Gerrit-Reviewer: Anuj Phadke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Tue, 16 Oct 2018 22:31:00 +0000 Gerrit-HasComments: Yes
