> On Oct. 21, 2014, 1:25 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > lines 817-819 > > <https://reviews.apache.org/r/26899/diff/1-2/?file=724971#file724971line817> > > > > FWIW "|=" is a standard way to record boolean True in a result. This > > feels lees readable in my opinion.
Sure thing, reverted. > On Oct. 21, 2014, 1:25 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 825 > > <https://reviews.apache.org/r/26899/diff/1-2/?file=724971#file724971line825> > > > > Would be great to update unit tests to validate this change. Which change are you thinking of? There's `testKillCronJob` to cover the cron case, if that's what you're referring to. Jacoco shows no branches missed here. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26899/#review57512 ----------------------------------------------------------- On Oct. 21, 2014, 1:09 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26899/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2014, 1:09 a.m.) > > > Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > Apologies for the large diff - `StateManager` has a large fan-out. This is a > pure functional no-op - just moving the call to `storage.write` up in the > stack. > > This change is to prepare for AURORA-871 [1], as well as generally make use > of `StateManager` safer. With the API prior to this diff, it would be easy > to unintentionally attempt a fateful read->write lock upgrade. This firms up > the API by requiring the caller to bring their own transaction, removing the > implementation detail that a write transaction is opened behind the scenes. > > [1] https://issues.apache.org/jira/browse/AURORA-871 > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java > 250c2df8113adfd62b3a7e124f7994156c82b5f7 > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java > e9f251508257cd7287ff00773e0073a3cd130df8 > src/main/java/org/apache/aurora/scheduler/async/TaskHistoryPruner.java > 345cd8959045302fe3711e22396f5f7244a88c44 > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java > 1189ed0318ae0cf9663f0fa41775c4dd625bb397 > src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java > ca6129c7e0225530336c88f91a1451892f2ce234 > src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java > 89140223f4f3acd02ade6fb95734744ef19d89bc > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java > 9388657ccb904364e460ec612c3da562b8952d7e > src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java > 21cfebdf11e0652e192cf08e35c8581b1246f7b5 > src/main/java/org/apache/aurora/scheduler/state/StateManager.java > 3a2fd279c2953d564b7fddabf31afda001bb3dfe > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > 58b94c2f2f3bac00f0692579974e8bdf159b6e40 > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java > 4db9be86f2e7db08d12e0182914a7c5130301b13 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > e792d23d6bb13b4e61b078beea6d063f72f0d8fc > src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java > 3774c851b58e747acf25735d24334408b1c5386d > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java > f4363aa8ce9ef9f583b52251f351c8c971ef8119 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 6ec130f4a9a5075b34452efb27c8fd0f08f93a63 > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java > 083a63543e5f9041f13fc6be66877f7173a5bf32 > src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java > 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 > src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java > b3e4ae39067b1dfb632f5d685d69fcbd7d4705da > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java > 6534329a92bf005223fa8907cbe4a8a3a511e142 > src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java > 9970948bace4c0ecbc51d6fc79270d77fb17bf87 > src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java > a28e512afde584fb94ff7686d7a3e3fbb51f8b7b > src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java > e79327c3b8ead01495e063e5c0e9270568e16f69 > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > b6b1bcbf8080eb7d1e7eca4a486cc063f28db75d > > src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java > 4d96761e04a342ad3564bdba4afdc889f27ac123 > src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java > cf4a015a040338a642fb07eec1fb7b5c11058fe5 > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > c48cbae4864127e7799917182439f7670285b0d3 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > f36a88fda3539553800bd727c3d2a77a54f1e71c > src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java > f9ed46f56bb11e9c158268c16f29557f3e99c84e > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > f739e6d1b1af6eea4875e03d32bfe88cef87b3ff > > Diff: https://reviews.apache.org/r/26899/diff/ > > > Testing > ------- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >