> On Sept. 3, 2014, 5:33 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java,
> >  lines 74-76
> > <https://reviews.apache.org/r/25300/diff/1/?file=675222#file675222line74>
> >
> >     This will also throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED 
> > -> ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift 
> > APIs (i.e. pause/resume/abort). Would it make sense to alter 
> > ALLOWED_TRANSITIONS with same state no-op transitions or throw here only if 
> > from != to?
> 
> Bill Farner wrote:
>     I didn't see much value in supporting no-op transitions.  The caller can 
> suppress that easily enough, so i chose to be more restrictive here.
> 
> Maxim Khutornenko wrote:
>     I guess I don't see how the caller would be able to set apart illegal 
> state transition (valid error) from the same state one. Are you suggesting 
> querying the state before trying to call into the controller?
> 
> Bill Farner wrote:
>     I'm suggesting that the caller avoid invoking transition(a, b) where 
> a==b.  If you'd prefer, i can allow that and return a NO_OP action, but all 
> that really does is move code around.  I don't have a strong opinion, neither 
> seems more attractive:
>     
>     ```
>     if (current != next) {
>       switch (transition(current, next)) {
>         case STOP_WATCHING:
>           ...
>       }
>     }
>     ```
>     
>     ```
>     switch (transition(current, next)) {
>       case NO_OP:
>         break;
>       case STOP_WATCHING:
>         ...
>     }
>     ```

Just realized this may be a case of different definitions of _caller_.  My 
_caller_ is the next level up in the call stack, sounds yours is an HTTP API 
consumer.


- Bill


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


On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25300/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2014, 4:02 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-613
>     https://issues.apache.org/jira/browse/AURORA-613
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This will be used by the job updater to gate insertion of JobUpdateEvents.
> 
> I chose not to use StateMachine here as it actually added complexity - in 
> this case we don't benefit from transition callbacks, and the result is based 
> only on the target state (rather than the transition).
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25300/diff/
> 
> 
> Testing
> -------
> 
> `./gradlew build -Pq`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to