Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8323 )

Change subject: IMPALA-1575: part 2: yield admission control resources
......................................................................


Patch Set 6: Code-Review+2

(1 comment)

If you do decide to migrate the test to HS2, have someone review those changes 
before carrying forward the +2.

http://gerrit.cloudera.org:8080/#/c/8323/5/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/8323/5/tests/custom_cluster/test_admission_controller.py@91
PS5, Line 91: ]
> Yeah I was checking the coverage bottom-up to make sure that all of the cod
We really should switch our pytests to use HS2 by default rather than beeswax, 
but that's clearly out of scope for this change.

Given how closely tied lifecycle is to the client API, it might be a good idea 
to see if you can easily move this test to HS2. I'll let you decide what the 
threshold for "easy" is, though. As you've reasoned, those last two cases are 
tied pretty closely to Close() and that's unlikely to change.



--
To view, visit http://gerrit.cloudera.org:8080/8323
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80279eb2bda740d7f61420f52db3bfa42a6a51ac
Gerrit-Change-Number: 8323
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: anujphadke <[email protected]>
Gerrit-Comment-Date: Mon, 06 Nov 2017 23:31:29 +0000
Gerrit-HasComments: Yes

Reply via email to