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

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:
    ...
}
```


- 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