> On Feb. 24, 2015, 7:27 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java,
> >  line 142
> > <https://reviews.apache.org/r/31235/diff/2/?file=873239#file873239line142>
> >
> >     rename to cronJobStore?

I will address this shortly in a separate diff along with storage.thrift struct 
renaming Bill suggested earlier.


> On Feb. 24, 2015, 7:27 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java,
> >  line 207
> > <https://reviews.apache.org/r/31235/diff/2/?file=873239#file873239line207>
> >
> >     rename to getCronJobStore?

same here


> On Feb. 24, 2015, 7:27 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java, 
> > line 63
> > <https://reviews.apache.org/r/31235/diff/2/?file=873240#file873240line63>
> >
> >     This TODO wasn't addressed - put it back?

The way I read this TODO is "don't fetch job configs, fetch job keys instead". 
Is that correct? If so, I don't think we will ever have a store interface to 
pull job keys. Since we store JobConfiugrations, pulling job keys will also 
require pulling JobConfigurations, so filtering job keys seems like a consumer 
thing to do.

Also, the code below appears to also require cron schedule rather than just a 
job key:
```
SanitizedCronJob cronJob = SanitizedCronJob.fromUnsanitized(job);
        cronJobManager.scheduleJob(
            cronJob.getCrontabEntry(),
            cronJob.getSanitizedConfig().getJobConfig().getKey());
```


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31235/#review73712
-----------------------------------------------------------


On Feb. 24, 2015, 1:23 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31235/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2015, 1:23 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Removing job store "proxy" methods from CronJobManager and dropping job 
> manager concept from the storage. The job manager is long gone but we still 
> carry around the job manager ID.
> 
> Despite the diff's size the changes are mostly mechanical: replacing 
> CronJobManager occurrences with direct job store access.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 31b698131aeb87ea0a633b0cee33b49ea1d501b4 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> b8830d187de27533307a8a2e9be6385f5d3e2289 
>   src/main/java/org/apache/aurora/scheduler/cron/CronJobManager.java 
> fc6e4432d64e625a583d8c8a130d99e066fd232c 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 
> efea7ad9f6efc99b600d071c3c20063b6bc4b211 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> c5cb8cae43c86ae2378a0bef7688d400aa188e57 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> 64d9486fade3b03b8db936fe60790ea0858212a9 
>   src/main/java/org/apache/aurora/scheduler/http/StructDump.java 
> 063170ac869dafc161c73f735b33f4cbe8e03ab6 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> dab8c0535bf9b36cdf7ca785c34d315f0bb4204c 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 4d83c58c8f0527813ef26477f08424104e0c29d9 
>   src/main/java/org/apache/aurora/scheduler/storage/JobStore.java 
> ad0d67a27628f46dedda2ae4e0e61025dff1e1fd 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 83a59ce5d5b2754b1d31354280eca31922d73cdd 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 52377bca8060720dc4bec884c911182c3e77bc52 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 384e9b536c3ee3edf7d90960aa51ef98948af088 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 27981393d700c84f6aaa791f12b24d0cec28b5df 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 262d72c781759bac58bfa6c0fcec66c513d8b79b 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java 
> c24a32e57b34a1fc41681fda9bdb4de38ed8896b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ee7618979ce94631af8aaf7ab3ecb2fbfb33fc38 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> 97ecb742d6e0418890f875394ded8d9fdae2b1c2 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 
> c99135ab9c55a42ab51f18cc5ea127b498f0721f 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 915d7c8294b3d8262021da1c30324f55d8413ff9 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  701536fff8948ef233523e114e45043992175891 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> b0772f73f1a21da5828660bfd7d2b1f6b15cbf74 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 493150b062eddf2581a048a9e13826205b8f2c15 
>   
> src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
>  479c8bded75c61c36ac70cf1d8f945b96850087e 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7e5d0edefbed3f67116361da15dd4c969c67cfe8 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
>  152e70489b351b5bcf06f85baddbe31259d0aef4 
>   src/test/java/org/apache/aurora/scheduler/storage/mem/MemJobStoreTest.java 
> 2e0c4151f58a28b695c13a392bae126857d4c4b6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  a85cba13fe7ed0e47072f391683261e4bec48009 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  945144dcb5240c3713d909344c82a9312cd3ba5c 
> 
> Diff: https://reviews.apache.org/r/31235/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to