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




src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (lines 351 - 356)
<https://reviews.apache.org/r/46810/#comment194993>

    These two cases are now mutually exclusive, use `else if` for the second?



src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 
45)
<https://reviews.apache.org/r/46810/#comment194995>

    nit: use a more appropriate variable name than `e` here to give some 
context (`r` or `resource`?).



src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 
67)
<https://reviews.apache.org/r/46810/#comment194996>

    Same here and below?
    
    (I notice this is fairly common, presumably `e` is short for "element" in 
these cases, but I find that giving it a more contextual name helps grok what's 
going on in the absence of explicit types, feel free to punt on this though if 
it's a major pain ;)).



src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java (line 
28)
<https://reviews.apache.org/r/46810/#comment194997>

    nit: just import `Protos.Offer`?



src/main/java/org/apache/aurora/scheduler/state/StateManager.java (line 18)
<https://reviews.apache.org/r/46810/#comment194998>

    Use `java.util.Function`?



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (line 111)
<https://reviews.apache.org/r/46810/#comment194999>

    this should be passing `assigned` to `mapAndAssign` not `task`, right? In 
case there are multiple resource types with mappers we don't want to discard 
any changes to the task from previous mappers.


- Joshua Cohen


On April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
> 
> (Updated April 29, 2016, 1:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A 
> few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any 
> resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within 
> `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and 
> assign port values
> - The newly introduced `ResourceManager` will eventually replace both 
> `Resources` and `ResourceSlot` classes.
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 625e6d5e1db1c7327154823f5a604efd3a4ff690 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 
> 1df2c11a84db8d46158c87e7c3709fc08016a107 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> 6e4d694cc6449a211297919d7834996382b21b21 
>   src/main/java/org/apache/aurora/scheduler/resources/Resources.java 
> 894c6a66545ac336d594d83c595ee83b7c91e21d 
>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 
> 5d34fe38492cae3f50ea9ed0baca11472295af60 
>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
> e5b2f41f55aec161840c3fb17f2ff73161b77482 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 7d43d4ad2de60f76183c91b860b00e7022d79bcb 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  731327950099f2567430f3af75633854910443ff 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  6370a126340dd4312ae0f2da52fee6ccdbced595 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> bf18d5d53f7eda62120299146a956fa0a0985f71 
>   src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java 
> d5f2172131ce4b765a1f85d63aeddf1daaa0b606 
>   src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 
> 04a82382e8535e1a233d48370bd0650f5abcd787 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java 
> ceb81e42e558fadb372cf4de862c6d3bcd293099 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
> 498da78169be42fd5edeb9963d8262af02895f0e 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> a2df3114a299218e7fb04a21cdedc8d911029bb0 
> 
> Diff: https://reviews.apache.org/r/46810/diff/
> 
> 
> Testing
> -------
> 
> unit and e2e tests
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to