-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19509/#review38147
-----------------------------------------------------------


Just FYI: the client unit tests weren't intended to detect stuff like this. The 
intention of those tests is just to verify that the client logic works - 
meaning that the client sends the API calls that we expect it to.  If an API 
gets changed in a way that doesn't change the API behavior of the client, then 
the client API unit tests aren't going to catch it.

There really should be tests beyond the simple API ones. For the updated, Maxim 
wrote some really nice logic tests that bypass the command-line parts, and 
verify that the API interaction logic is correct. That would be a good model to 
follow.

As far as the thermos stuff goes, I don't see anything in this change that 
should cause a thermos problem - but I don't know nearly enough about thermos, 
so I'll defer to Brian, who's the thermos God.




- Mark Chu-Carroll


On March 20, 2014, 10:39 p.m., Suman Karumuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19509/
> -----------------------------------------------------------
> 
> (Updated March 20, 2014, 10:39 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman.
> 
> 
> Bugs: AURORA-246
>     https://issues.apache.org/jira/browse/AURORA-246
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Updated the client to consume the new getQuota API which contains 
> nonProdConsumption(resources consumed by non-prod tasks) information also. 
> 
> Currently, the mocked calls can't detect renamed and missing thrift structs. 
> Added tests to look for expected fields in GetQuotaResult and  
> ResourceAggregate structs. Refactored tests in test_quota to remove 
> duplicated code and added a missing test.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/quota.py 
> d06f21a80575058aefa3dffc72b365805d7a5ce2 
>   src/main/python/apache/aurora/client/commands/core.py 
> 9977c725528086d3e8cf58de294adee542570411 
>   src/test/python/apache/aurora/client/cli/test_quota.py 
> 44afd74aa5b11296951f45fe7edca8cb58b0ec18 
> 
> Diff: https://reviews.apache.org/r/19509/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python:all -vxs 
> 
> Works for all quota related test. 
> src.test.python.apache.thermos.bin.test_thermos fails on my laptop because of 
> build issues. Sending out a code review since it is an un-related issue. Will 
> look into this tomorrow, if it's a blocker.
> 
> Found all the code to update by running the following query.
> 
> ?  git:(mansu/AURORA-246_getQuotaAPI) ag getQuotaResult -G '.py'              
>                                                                           
> ~/workspace/incubator-aurora
> src/main/python/apache/aurora/client/api/quota_check.py
> 90:    allocated = CapacityRequest(resp.result.getQuotaResult.quota)
> 91:    consumed = CapacityRequest(resp.result.getQuotaResult.prodConsumption)
> 
> src/main/python/apache/aurora/client/cli/quota.py
> 65:      return serialize(quota_resp.result.getQuotaResult,
> 69:      result += get_quota_str(quota_resp.result.getQuotaResult.quota)
> 70:      if quota_resp.result.getQuotaResult.prodConsumption:
> 72:        result += 
> get_quota_str(quota_resp.result.getQuotaResult.prodConsumption)
> 73:      if quota_resp.result.getQuotaResult.nonProdConsumption:
> 75:        result += 
> get_quota_str(quota_resp.result.getQuotaResult.nonProdConsumption)
> 
> src/main/python/apache/aurora/client/commands/admin.py
> 199:  quota = resp.result.getQuotaResult.quota
> 
> src/main/python/apache/aurora/client/commands/core.py
> 632:  print_quota(resp.result.getQuotaResult.quota, 'Total allocated quota', 
> role)
> 634:  if resp.result.getQuotaResult.prodConsumption:
> 635:    print_quota(resp.result.getQuotaResult.prodConsumption,
> 639:  if resp.result.getQuotaResult.nonProdConsumption:
> 640:    print_quota(resp.result.getQuotaResult.nonProdConsumption,
> 
> src/test/python/apache/aurora/client/api/test_quota_check.py
> 49:        getQuotaResult=GetQuotaResult(
> 
> src/test/python/apache/aurora/client/cli/test_quota.py
> 35:    response.result.getQuotaResult = GetQuotaResult()
> 36:    response.result.getQuotaResult.quota = ResourceAggregate()
> 37:    response.result.getQuotaResult.quota.numCpus = 5
> 38:    response.result.getQuotaResult.quota.ramMb = 20480
> 39:    response.result.getQuotaResult.quota.diskMb = 40960
> 40:    response.result.getQuotaResult.consumed = None
> 47:    response.result.getQuotaResult = GetQuotaResult()
> 48:    response.result.getQuotaResult.quota = ResourceAggregate()
> 49:    response.result.getQuotaResult.quota.numCpus = 5
> 50:    response.result.getQuotaResult.quota.ramMb = 20480
> 51:    response.result.getQuotaResult.quota.diskMb = 40960
> 52:    response.result.getQuotaResult.prodConsumption = ResourceAggregate()
> 53:    response.result.getQuotaResult.prodConsumption.numCpus = 1
> 54:    response.result.getQuotaResult.prodConsumption.ramMb = 1024
> 55:    response.result.getQuotaResult.prodConsumption.diskMb = 2048
> 56:    response.result.getQuotaResult.nonProdConsumption = ResourceAggregate()
> 57:    response.result.getQuotaResult.nonProdConsumption.numCpus = 1
> 58:    response.result.getQuotaResult.nonProdConsumption.ramMb = 1024
> 59:    response.result.getQuotaResult.nonProdConsumption.diskMb = 2048
> ?  git:(mansu/AURORA-246_getQuotaAPI)                                         
>                                                                           
> ~/workspace/incubator-aurora
> 
> 
> Thanks,
> 
> Suman Karumuri
> 
>

Reply via email to