> On Aug. 30, 2015, 3:20 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 74 > > <https://reviews.apache.org/r/37825/diff/5/?file=1059384#file1059384line74> > > > > You might consider killing the TODO. There probably isn't a great way > > to link to the correct documentation, as it would really need to be > > associated with the version.
Done. > On Aug. 30, 2015, 3:20 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/TierInfo.java, line 24 > > <https://reviews.apache.org/r/37825/diff/5/?file=1059385#file1059385line24> > > > > Totally nit-picking - `TierInfo.DEFAULT_TIER` is a bit redundant, as > > the containing class and the field type already contain 'tier'. Consider > > `TierInfo.DEFAULT`, and skip the static import where it's used to keep the > > context. Done. > On Aug. 30, 2015, 3:20 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/TierManager.java, line 55 > > <https://reviews.apache.org/r/37825/diff/5/?file=1059386#file1059386line55> > > > > You could avoid the supplier/memoization and instead use a traditional > > constructor with annotations: > > > > ``` > > static final TierConfig EMPTY = new TierConfig(ImmutableMap.of()); > > > > private final Map<String, TierInfo> tiers; > > > > @JsonCreator > > TierConfig(@JsonProperty("tiers") Map<String, TierInfo> tiers) { > > this.tiers = ImmutableMap.copyOf(tiers); > > } > > ``` > > > > I like this since the 'JSON-ness' is completely isolated to the > > constructor. Happy to accept. Frankly, jackson json tuning is such a PITA! > On Aug. 30, 2015, 3:20 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/TierManager.java, line 50 > > <https://reviews.apache.org/r/37825/diff/5/?file=1059386#file1059386line50> > > > > You could avoid the annotation with this mapper setting: > > ``` > > ObjectMapper mapper = new ObjectMapper(); > > mapper.setVisibilityChecker( > > mapper.getVisibilityChecker().withFieldVisibility(Visibility.ANY)); > > ``` > > > > However, the advice below would also remove it and is IMHO a slightly > > better approach. Took advice below. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37825/#review97027 ----------------------------------------------------------- On Aug. 29, 2015, 6:34 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37825/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2015, 6:34 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-1437 > https://issues.apache.org/jira/browse/AURORA-1437 > > > Repository: aurora > > > Description > ------- > > The external config file is optional for now as tiers are not fully defined > yet. > > > Diffs > ----- > > examples/vagrant/upstart/aurora-scheduler-kerberos.conf > 744b4a35c61e749734e222b3d4cbd296927665aa > examples/vagrant/upstart/aurora-scheduler.conf > 789a3a0315e8530880999432aa9b1e7d0f57d1ff > src/jmh/java/org/apache/aurora/benchmark/Offers.java > 26bad1cce987ef7f46368a5936cf056aeb2f63b1 > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java > d4bc6b9bb10c982fb6a2458d7fdd12bddbe98eca > src/main/java/org/apache/aurora/scheduler/TierInfo.java > 61bf30a7f11d2d4b9e49c58a6ed9ecd779d7e5ba > src/main/java/org/apache/aurora/scheduler/TierManager.java > 652afece17a7eb09b0ca68066707b1b8fbf024f0 > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > d7f3c60d383cf10afb0c0fcf4fe29972b183458c > src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java > 399f58de6196b97abd359ecef8131b63480d591a > src/test/java/org/apache/aurora/scheduler/ResourcesTest.java > eb1baedcb13c2f169d819d137f22cb8b88db149c > src/test/java/org/apache/aurora/scheduler/SchedulerModuleTest.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > eb0207114e1b93968cb65832f154a9cd3bc3232e > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > adcc7a823ecf30442016ecbdd655622d6aeba65e > > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java > 819b51e4c314749dc48db25693503af7d1ed0c54 > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > d8a524d98771ee68d7b4d423fb34e28101a04d27 > src/test/resources/org/apache/aurora/scheduler/tiers-example.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/37825/diff/ > > > Testing > ------- > > ./gradlew -Pq build > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Maxim Khutornenko > >
