-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24465/#review50038
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87549>

    Missing @param <K> comment.



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87553>

    How would instance events propagate from here? From what I can see the 
caller would not have the instance state exposure, so I am assuming it's going 
to be another "instanceEventSink" constructor argument?



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87552>

    This method is a bit hard to follow given the presence of both update and 
instance state machine transitions. How about encapsulating instance 
transitions in helper methods? Would make the algorithm here more readable.



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87554>

    This would make it impossible to do a failure-agnostic rollback we 
currently have in client. Are you suggesting the rollback would be a best 
effort aborting in case of maxFailedInstances reached?



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java
<https://reviews.apache.org/r/24465/#comment87555>

    I thought the thrift JobUpdateAction is what we would use here, no?


- Maxim Khutornenko


On Aug. 7, 2014, 11:11 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24465/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 11:11 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-613
>     https://issues.apache.org/jira/browse/AURORA-613
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> There are 3 levels to performing an update:
> 
> 1. Move the job from state A to state B, roll back on failure
> 2. Take a job from state A to state B
> 3. Take an instance from state A to state B
> 
> This implements level 2.  I made the OneWayJobUpdater generic, which actually 
> simplified both implementation and testing, since it is only responsible for 
> relaying that state down to level 3.
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceStateProvider.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> 7476d82e9691449fa968c5fc4c5af76837a5c9cf 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 
> dda1b73ead847e5ee9c5c7bc8be3cd8a7f59ac80 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24465/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> 
> OneWayJobUpdater has 100% instruction and branch coverage.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to