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