impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1575: part 2: yield admission control resources
Date Mon, 06 Nov 2017 23:31:29 GMT
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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Nov 2017 23:31:29 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message