> On March 16, 2015, 4:30 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java, > > line 159 > > <https://reviews.apache.org/r/32106/diff/1/?file=895820#file895820line159> > > > > There is actually a better place to do this type of action [1]. If not > > done there, the update will still have to iterate over all instances and > > generate a lot of instance event noise. Also, the UI will show these > > instances in the update working set, which does not reflect the NOOP nature > > of this action. > > > > [1] - > > https://github.com/apache/incubator-aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java > > Bill Farner wrote: > Good point. Steve landed at this line based on my hasty direction. > You're right, though, that JobDiff is the place to move it. Perhaps this > comparison should be done in JobDiff as well, to prevent impedance mismatch?
Also, better yet - a test case in `JobUpdaterIT` should have pointed out this flaw. Steve - can you add a case there? - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/#review76568 ----------------------------------------------------------- On March 16, 2015, 3:13 p.m., Steve Niemitz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32106/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 3:13 p.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-1200 > https://issues.apache.org/jira/browse/AURORA-1200 > > > Repository: aurora > > > Description > ------- > > Changed the updater to not update an instance if the job owner changes > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java > 14753cf5ef35d5133ca5029f3a465df884756070 > src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java > 09c147e76d7c2c130a1fdd85459c45395fee7dde > > Diff: https://reviews.apache.org/r/32106/diff/ > > > Testing > ------- > > > Thanks, > > Steve Niemitz > >
