Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11540 )
Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py@81 PS3, Line 81: float(client_connect_timeout_ms) Any idea what happens if the user passes a negative value ? Also, it seems a bit weird that it's a float. Does int not work ? http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py@a864 PS3, Line 864: Why was this removed ? http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225 PS3, Line 225: "-t", "--client_connect_timeout_ms" Do you think it makes sense to have a value (e.g. 0) which allows user to disable the timeout behavior ? Actually, have you confirmed the behavior of setting this to 0 ? Does it timeout all the time ? http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@227 PS3, Line 227: if nit: missing space before "if" http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@740 PS3, Line 740: nit: extra whitespace http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@742 PS3, Line 742: indefinately indefinitely ? -- 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: 3 Gerrit-Owner: anujphadke <apha...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: anujphadke <apha...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Oct 2018 01:07:22 +0000 Gerrit-HasComments: Yes
