> On Aug. 18, 2015, 8:08 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/TierInfo.java, line 27
> > <https://reviews.apache.org/r/37560/diff/2/?file=1043194#file1043194line27>
> >
> >     For better readability, how about using an enum?
> 
> Maxim Khutornenko wrote:
>     Not sure I undestand. Are you proposing converting TierInfo into an enum 
> or the arguments of TierInfo to be an EnumSet? I am afraid neither would make 
> sense as TierInfo will eventually wrap free-form task attributes defined in 
> an external config file. Given the config file example in 
> https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo,
>  a possible TierInfo constructor could look similar to this:
>     ```
>     TierInfo(String name, boolean revocable, boolean preemtible, boolean 
> quotaRequired, Amount<Long, TimeUnit> uptime, double percentage ...)
>     ```
> 
> Stephan Erb wrote:
>     I was referring to the boolean values which IMHO make constructor calls 
> (for example in tests) very hard to read. For example: `TierInfo('mytier', 
> false, false, true, ...)` vs `TierInfo('mytier', Offers.UNREVOCABLE, 
> Jobs.UNPREEMPTIBLE, Quotas.REQUIRED, ...)`.
>     
>     But you are right that this adds an aditional indirection layer when 
> passing values from the config.
> 
> Maxim Khutornenko wrote:
>     Got it. Yeah, I'd avoid adding indirection like that, given that the 
> TierInfo constructor is only intended to be called from TierManager.

Ok then, thanks the update :-)


- Stephan


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


On Aug. 18, 2015, 8:23 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37560/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 8:23 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1437
>     https://issues.apache.org/jira/browse/AURORA-1437
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Introducing `TierManager` implementation. This diff addresses basic needs 
> within the Oversubscription feature scope.
> 
> Only TaskAssigner is modified to use TierManager. Other consumers will be 
> addressed separately.
> 
> More about the TierManager concept here: 
> https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/Resources.java 
> 40df26234ad9959227d8a56edbe563f252d47560 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 45265ea6cef5e26a3d7303a9653cfca110528d4b 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> ca4b5b0b01c7b347fc90f40d77f96cf9d4d94163 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> a5878a4efada5b5872be77ad887a0ffe8695d6b8 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 88958d1510cf9402a7d1e422de9f7918004e188b 
> 
> Diff: https://reviews.apache.org/r/37560/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to