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




src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (lines 344 - 345)
<https://reviews.apache.org/r/56395/#comment238307>

    I belive this will not work as expected if a resource type is missing. 
`getResource` throws `IllegalArgumentException` but users of the this class 
expect we throw `TaskDescriptionException`.
    
    My recommendation would be:
    * leave `getResource` in the `ThriftBackFill` class. It is scrape code that 
we will remove soon anyway.
    * Leverage the already existing `ResourceManager` methods here instead: 
`quantityOf(getTaskResources(config, CPUS))`


- Stephan Erb


On Feb. 7, 2017, 6:02 p.m., Nicolás Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 6:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The Resource validation in ConfigurationManager is now done against the 
> Resource set instead of the NumCpus, RamMb and DiskMb fields.
> 
> To do this I moved the GetResource method from the ThriftBackfill to the 
> ResourceManager so it can be used in the ConfigurationManager without 
> exposing it on the ThriftBackfill.
> 
> Related Issue: Aurora-1707
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  80f0aebd6de0c2d7c30a58ec09702d1c6d519787 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
> 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> Testing
> -------
> 
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Nicolás Donatucci
> 
>

Reply via email to