----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25969/#review54419 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java <https://reviews.apache.org/r/25969/#comment94545> s/_get_/_fetch_ to match method name. src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java <https://reviews.apache.org/r/25969/#comment94549> How about moving this into a static helper in JobDiff for better readability? src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java <https://reviews.apache.org/r/25969/#comment94550> Suggest a more descriptive message here mentioning updateOnlyTheseInstances property name. src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java <https://reviews.apache.org/r/25969/#comment94558> Does this translate into INVALID_REQUEST in thrift? Not sure it's a good idea unless we expose a jobDiff RPC to reliably compare job configs before the update. Otherwise, it's client diff vs. server diff with a chance of mismatch and subpar client experience. Should it rather be special-cased to return OK with a detail message stating a noop update? src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java <https://reviews.apache.org/r/25969/#comment94561> Unless I am reading it wrong, I am not sure how this could even be possible now that noop instances are rejected by the Diff. Is there still a case for it? src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml <https://reviews.apache.org/r/25969/#comment94546> Why left join here? Events without update ID would not make any sense. src/main/thrift/org/apache/aurora/gen/api.thrift <https://reviews.apache.org/r/25969/#comment94547> s/../. - Maxim Khutornenko On Sept. 24, 2014, 12:44 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25969/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2014, 12:44 a.m.) > > > Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. > > > Bugs: AURORA-718 > https://issues.apache.org/jira/browse/AURORA-718 > > > Repository: aurora > > > Description > ------- > > When creating an update, store only the delta between the initial and desired > states. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/base/Numbers.java > 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 > src/main/java/org/apache/aurora/scheduler/base/Tasks.java > 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d > src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java > b7143941feb4824aee609741e8e2c0e015eecac3 > src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java > 1874a3e3f9bc47104b7597fd88d77c0c36620f7d > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java > 15a9e2b9acd65f6268e70fb402414f5c01c42409 > > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java > 26cc85dcb6dccd913e9a130a134fbf8df10cc08d > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > ae0320b44b55a3630e255484ea7a881daab6a7bc > src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > 348bdbfbb51800d31b67ea1b3f2632975e598306 > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java > 1d0bbfef583708bfdab30802724273e4b21ecf8f > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 842256a16fe2775af865ed2ee91fd719b3476577 > src/main/python/apache/aurora/client/api/__init__.py > bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 > > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml > e1179b449c05adc56206bfef271a9d440a77e042 > src/main/thrift/org/apache/aurora/gen/api.thrift > 2376a5e530b12fbbebb4cfc7555656ae07795518 > > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java > 553ef4d88fafff894a11bc6fb6d5ab33d5f6646d > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 > src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > f7c7205d80edac714a12c2aff0cc7db27b1909b4 > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java > ad4040c4a58d3e6f075a60712144aa6968270a55 > > src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java > 548d31580b5f2bae92900fddb80005f7d274a155 > > Diff: https://reviews.apache.org/r/25969/diff/ > > > Testing > ------- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >