> On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java, line
> > 120
> > <https://reviews.apache.org/r/24815/diff/1/?file=662756#file662756line120>
> >
> > At first glance, this is odd since ITaskConfig contains the components
> > of a JobKey. Is that argument necessary?
>
> Maxim Khutornenko wrote:
> I tried and really hated that approach. Adding a bit of redunancy helped
> avoiding clumsy job key extraction and edge case error conditions (e.g. empty
> collection, ITaskConfigs from different jobs and etc.)
> avoiding clumsy job key extraction and edge case error conditions
Those cases are still there though, right? Except now there's an additional
case currently not handled where the keys don't match. Perhaps the better
approach is to change the signature to:
`insertPendingTasks(ITaskConfig config, Set<Integer> instanceIds);`
Looking at the call sites, this is how it's used in practice.
In fact, there's a relevant TODO in SanitizedConfiguration:
`// TODO(William Farner): Rework this API now that all configs are identical.`
Even the late SchedulerCore#addInstance took this approach:
```java
public void addInstances(
final IJobKey jobKey,
final ImmutableSet<Integer> instanceIds,
final ITaskConfig config) throws ScheduleException {
```
(though unfortunately it also duplicated the job key)
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24815/#review52089
-----------------------------------------------------------
On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24815/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2014, 8:04 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/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
> 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f
> 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
> cc1eee4e19c092c0d401558ac01b54627f2d1290
> src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java
> PRE-CREATION
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
> 7ef28858ad290c74248b89c49d2a684eb1c7127e
>
> 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
> fa611a913bad40a8c0515c578b394c460340e574
> 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/mem/MemStorageSchedulerCoreImplTest.java
> 35bed104d838596abcbb5abd5cad29592b384dfa
>
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
> 649afa24b2cfc9a1d67d350473e439d209bd720c
> src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
> 43265fdab1ae900fb828374f6c69e562def2d682
>
> Diff: https://reviews.apache.org/r/24815/diff/
>
>
> Testing
> -------
>
> gradle -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
