----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117808 -----------------------------------------------------------
Fix it, then Ship it! src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java (line 45) <https://reviews.apache.org/r/43172/#comment179074> Having a backfill in the db layer strikes me as a red flag; ie all thrift inputs to the scheduler should be sanitized as close as possible to the input site. I think this means asap in a thrift RPC and when reading from the distributed log. This may be impractical to implement today or else this just may be more expedient to getting 0.12.0 out the door with confidence in which case I'm fine with it, but I'd like to file a follow-up issue to investigate a generic intercepting layer for the thrift RPC interface and the LogStorage that could look up thrift structs types in a map and apply a sanitization UnaryOperator if found... mod some handwaving. src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java (line 50) <https://reviews.apache.org/r/43172/#comment179072> Finish this thought - there is no actual testing done here. - John Sirois On Feb. 3, 2016, 7:14 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43172/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2016, 7:14 p.m.) > > > Review request for Aurora, John Sirois and Bill Farner. > > > Bugs: AURORA-1603 > https://issues.apache.org/jira/browse/AURORA-1603 > > > Repository: aurora > > > Description > ------- > > This change ensures we can un-revert https://reviews.apache.org/r/43104/ when > 0.12.0 ships and still have a graceful rollback from 0.13.0 to 0.12.0. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java > 27f1f33ffc782b3d2a2a9add494c04911659e217 > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > db90150b48c5b134dde6c69f70ebca82bfdd0c12 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java > 54defc256ad8a261c6b56ee06ad7fdd16a26b057 > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > 7382eca281eeab17d407ed140f16d6a633d8ad72 > > src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java > 806f50d57e44261e3a1988a3c9bb742442badde8 > > src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/43172/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >