Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 )
Change subject: IMPALA-6591: Fix test_ssl flaky test ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/12047/2/tests/custom_cluster/test_client_ssl.py File tests/custom_cluster/test_client_ssl.py: http://gerrit.cloudera.org:8080/#/c/12047/2/tests/custom_cluster/test_client_ssl.py@88 PS2, Line 88: p = ImpalaShell(args="--ssl", wait_until_connected=True) > I added a new parameter in the constructor to wait until the shell is ready How many places would these be? I think the behavior should really be wait by default and let tests opt out if they have to. Given we already changed a few assertions in unrelated tests, we should consider making this the default. http://gerrit.cloudera.org:8080/#/c/12047/4/tests/shell/util.py File tests/shell/util.py: http://gerrit.cloudera.org:8080/#/c/12047/4/tests/shell/util.py@128 PS4, Line 128: Trying to read : # from stderr when there is nothing to read will block the readline() operation : # indefinitely. I don't think this second sentence is necessary. http://gerrit.cloudera.org:8080/#/c/12047/4/tests/shell/util.py@132 PS4, Line 132: while "Connected to" not in self.shell_process.stderr.readline(): This should have a timeout. It can be high (5min) but that will make a hang less likely -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Comment-Date: Mon, 10 Dec 2018 19:16:39 +0000 Gerrit-HasComments: Yes
