> 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 ...) > ```
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. - 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 > >