----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41145/#review109612 -----------------------------------------------------------
Ship it! It took me a few passes to understand why this change was logically correct. Dumping my analysis - turns out the root of the problem is that we are essentially applying the update scope _twice_, which produces incorrect results. Maxim's change is meaningful because `JobUpdateInstructions.desiredState` already describes actions to take based on `JobUpdateInstructions.settings.updateOnlyTheseInstances`. - Bill Farner On Dec. 9, 2015, 10:23 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41145/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2015, 10:23 a.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-1549 > https://issues.apache.org/jira/browse/AURORA-1549 > > > Repository: aurora > > > Description > ------- > > Changed updater factory to initialize instance updaters based on the union of > initial & desired states, which are now populated exclusively off of JobDiff > results. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 44402c198a75c32f8f8fd5a193057d12adec21f5 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > d98797406d9d5e9de10dbacc7640e513e291b2ff > > src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java > 0b193c41d2c6e1455f5e290286c65bba3b6fc3f5 > > Diff: https://reviews.apache.org/r/41145/diff/ > > > Testing > ------- > > ./gradlew -Pq build > Manual testing in vagrant. > > > Thanks, > > Maxim Khutornenko > >
