> On Sept. 5, 2014, 3:25 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, > > line 47 > > <https://reviews.apache.org/r/24815/diff/2/?file=679138#file679138line47> > > > > s/instances/newInstances/? > > > > It would be nice for the parameter name and doc to call out that the > > validation is based on things being _added_ to the existing task pool. > > Maxim Khutornenko wrote: > | It would be nice for the parameter name and doc to call out that the > validation is based on things being added to the existing task pool. > > This is not entirely true. It's also called from createJob() when the > task pool techincally does not exist yet. > > However, your comment points to a potential flaw with MAX_TASKS_PER_JOB > check. It does not cover the addInstances case. It's currently possible to > create an unbounded number of job instances via subsequent job updates with > newInstances <= MAX_TASKS_PER_JOB. Added a TODO and will address this in a > follow up review. > > Bill Farner wrote: > > It's also called from createJob() when the task pool techincally does > not exist yet. > > Understood, but if the nouns point out that these are _new_ tasks, it > will still be valid in that case. > > In other words, i could read the current signature as "validate the > limits of the proposed new total pool of tasks". Rather than "validate the > limits if these proposed tasks are added".
Agree. I changed the nouns in my last diff. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/#review52455 ----------------------------------------------------------- On Sept. 5, 2014, 8:26 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24815/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2014, 8:26 p.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-94 > https://issues.apache.org/jira/browse/AURORA-94 > > > Repository: aurora > > > Description > ------- > > Moving the last bits of functionality out of SchedulerCore. > > Also, preparing the ground for task validation logic reuse in updater code. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java > e060e5ec88a0ab311415eaa638cf693c99c40049 > > src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java > d511ec0054e380fd28b0c70bae7d9803b81abdf1 > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java > 62202810fd5829545fc77b91a20d1bb433a4916a > src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java > c636fd7fde8b592b167da8e5e9651ac772bc23de > src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java > 1e60fcad82b05e3fe477a31e653b8c5e63c78050 > src/main/java/org/apache/aurora/scheduler/state/StateManager.java > 6e062b39175255264020bbb1cbd485de9a114a20 > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > 6ad104b050aabecad1dadc975c27d9d3603659bf > src/main/java/org/apache/aurora/scheduler/state/StateModule.java > 2c712eff097c3334bfcf2559a52214367748d08a > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 9171179ff482e0b56faf4073b2dc2a6f2f0def46 > src/main/thrift/org/apache/aurora/gen/api.thrift > a78a4d849e545000725a39fae49f406422c39da0 > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > c7ae7db16e82c722fae1bccb9178f5ec203e91e5 > src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java > 62dce07b42af02d25b788e763e4e2a026dd2d483 > > src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java > b22b390368e08d3790480ab5505852f398b2c69c > src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java > 1678411ad5c25e74f8bbbbb6d004bf491ea26ca0 > src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java > ad2548c53d86b89efe6129702b8a5df259af3d4e > > src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java > 35bed104d838596abcbb5abd5cad29592b384dfa > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 1686d4b158057d87180af3a211a4237f1213efa6 > src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java > 055c177bc34cc306faaf7593e6c5156ad9636f6c > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 81d1734a22a8744d6002aadb7fb446d132d10bd9 > > Diff: https://reviews.apache.org/r/24815/diff/ > > > Testing > ------- > > gradle -Pq build > > > Thanks, > > Maxim Khutornenko > >
