> On Nov. 17, 2014, 9:48 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, > > line 108 > > <https://reviews.apache.org/r/28103/diff/2/?file=765455#file765455line108> > > > > I'm a little worried that we have to add yet another class to represent > > resources in the cluster. > > > > It's not blocking this review but I would like to know your thoughts on > > this.
This type of new encapsulation is a good thing for future consolidation, as it calls out a previously-hidden use case. That said, we should avoid consolidation just because of a noun clash - it should become obvious when two disparate types are serving the same purpose. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28103/#review61818 ----------------------------------------------------------- On Nov. 16, 2014, 10:29 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28103/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2014, 10:29 p.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Repository: aurora > > > Description > ------- > > Simplify Preemptor code, encapsulate fields used there and in > SchedulingFilter. > > A lot of this addressing Law of Demeter violations, such as accepting > `IAssignedTask` when only `ITaskConfig` and task ID were needed. There are > also (what i consider) readability improvements in `SchedulingFilterImpl` and > `PreemptorImpl`. > > The broader goal here is to simplify the code usedin scheduling, hopefully to > make forthcoming scheduling performance improvements less complicated. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java > ff26c49729646ffe052cb0a993b9984ae96a89ac > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java > 6bfa3ac425ed3045fa60d1b0ca547e9bf3cde37a > src/main/java/org/apache/aurora/scheduler/base/Tasks.java > a2997f518f90eac34cb6fbb1104240b823d45f22 > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 8cf845f3622392a65216e0c29084965c7c64075d > > src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java > ca53303a675be60300cc1b6534164fa6da7ddbd7 > src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java > 3839083f27ca5d4b93406152559b58b04e912a10 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java > c1c5f26723f1eac3000e09e061b4582f922fded6 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java > cc6b53b3265253f76c1e954c0108aa5936f5cc36 > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java > 4abc7ba36c547624af51fabc0983099efe5798ea > src/main/java/org/apache/aurora/scheduler/stats/ResourceCounter.java > 79d12b0dd7959b5443ffce43d9ebdb79135718bb > src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java > 8b0367ec99701084ce0cf55229a363c4b0b66b8f > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java > 9bc6a7535bf69dbc19771aa1834aeb04f42eea48 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java > d1bc9bfe987b83356483cf1fb04aef2eb51eb141 > > src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java > 94f0a179b786649775899f855f7c1a0caab7290f > > src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java > e113eba1f304279b5ee3d70db1d1ea558efd63ac > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > a8a70b65c6f91371b9afad4dd3806a7c86fba04f > > Diff: https://reviews.apache.org/r/28103/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
