> On Oct. 28, 2014, 5:30 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java, > > line 129 > > <https://reviews.apache.org/r/27262/diff/2/?file=735497#file735497line129> > > > > Can you fully centralize the backfill and the counter increment? > > > > void populateJobKey(TaskConfig config) { > > if job key is valid: > > fill job key > > increment counter > > } > > Maxim Khutornenko wrote: > I can move the counter in but not the validation check, which goes > against the IJobKey.
Why not `IJobKey.build(config.getJob())`? Given that we've already had one non-obvious bug with this code, i think it's prudent to not replicate it. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27262/#review58828 ----------------------------------------------------------- On Oct. 28, 2014, 6:01 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27262/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2014, 6:01 p.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-899 > https://issues.apache.org/jira/browse/AURORA-899 > > > Repository: aurora > > > Description > ------- > > Adding missing JobConfiguration backfill for task config job field. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java > 97517ed609289f584e07620d24f6fc09c54d36fc > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java > 32a90d5a7819404a3d9e0b94168a447398607e56 > > Diff: https://reviews.apache.org/r/27262/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
