Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 )
Change subject: IMPALA-6591: Fix test_ssl flaky test ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/12047/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12047/2//COMMIT_MSG@13 PS2, Line 13: met. In other words, the logic in test_ssl that loops while the number > can you point to where that logic is? ImpaladService.wait_for_num_in_flight https://github.com/apache/impala/blob/master/tests/custom_cluster/test_client_ssl.py#L99-L104, if https://github.com/apache/impala/blob/master/tests/custom_cluster/test_client_ssl.py#L91 is false, that while loop impalad.get_num_in_flight_queries() returns 0, hence the whole block: https://github.com/apache/impala/blob/master/tests/custom_cluster/test_client_ssl.py#L99-L104 will be skipped. 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) > Can we change the constructor to return only when the shell is ready to acc I added a new parameter in the constructor to wait until the shell is ready. Making it a default behavior requires updating in many places since some tests need read the "Connected to:" message from stderr. http://gerrit.cloudera.org:8080/#/c/12047/2/tests/custom_cluster/test_client_ssl.py@93 PS2, Line 93: LOG = logging.getLogger('test_client_ssl') > I think that wait_for_num_in_flight_queries() would be more intuitive if it I added assert in places that call wait_for_num_in_flight_queries. -- 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: 3 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Comment-Date: Sat, 08 Dec 2018 02:21:42 +0000 Gerrit-HasComments: Yes
