Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12047 )

Change subject: IMPALA-6591: Fix test_ssl flaky test
......................................................................


Patch Set 2:

(1 comment)

I agree with Lars's comments + see my own, but I am ok with the patch as it is 
if the broken test needs urgent fix.

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@93
PS2, Line 93:     impalad.wait_for_num_in_flight_queries(1)
> This method actually returns a boolean on whether it reached the required n
I think that wait_for_num_in_flight_queries() would be more intuitive if it 
would assert by default on timeout. A try_wait_for_num_in_flight_queries() 
version could be added with the current behavior.

I grepped around and 6 of the 12 usages of this function call it without 
checking return value, while the context seems to assume it always succeeds.



--
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: 2
Gerrit-Owner: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Comment-Date: Fri, 07 Dec 2018 16:48:10 +0000
Gerrit-HasComments: Yes

Reply via email to