Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14059 )
Change subject: IMPALA-8863: Add support to run tests over HTTP/HS2 ...................................................................... Patch Set 14: (4 comments) http://gerrit.cloudera.org:8080/#/c/14059/14/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/14059/14/tests/common/impala_connection.py@323 PS14, Line 323: if 'NoneType' not in str(e): Can we restrict this to HTTP connections only? Would be nice not to swallow exceptions unless necessary. http://gerrit.cloudera.org:8080/#/c/14059/14/tests/common/test_dimensions.py File tests/common/test_dimensions.py: http://gerrit.cloudera.org:8080/#/c/14059/14/tests/common/test_dimensions.py@117 PS14, Line 117: # IMPALA-8864: Older python versions do not support SSLContext object that the thrift http client nit: long line http://gerrit.cloudera.org:8080/#/c/14059/14/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/14059/14/tests/shell/test_shell_interactive.py@293 PS14, Line 293: self.create_impala_clients() Should this be in a finally? So that a failure here doesn't cause follow-on effects. Probably OK to ignore, would just reduce noise if the test fails. http://gerrit.cloudera.org:8080/#/c/14059/14/tests/shell/test_shell_interactive.py@335 PS14, Line 335: self.create_impala_clients() Same here (I think I created this problem in the first place by closing the clients and not reopening, under the assumption that they would be re-created). I guess the other way we could solve is by reopening the clients automatically in the test setup if they were closed. But don't need to do that. -- To view, visit http://gerrit.cloudera.org:8080/14059 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7156558071781378fcb9c8941c0f4dd82eb0d018 Gerrit-Change-Number: 14059 Gerrit-PatchSet: 14 Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 28 Sep 2019 00:25:52 +0000 Gerrit-HasComments: Yes
