-----------------------------------------------------------
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
> 
>

Reply via email to