TaskSpec refactoring

[knipknap]

Some refactoring of the TaskSpec classes seems to be in order:

• Is TaskSpec._predict_hook required? Does a simpler alternative exist?
• Stop interpreting the return value of hook functions, it makes things hard to understand.
• Predicted states should ONLY be set within the predict() method, nowhere else.
• The TRIGGERED state should be an extra flag instead of part of the state value.
• my_task._inherit_attributes() in TaskSpec should be called in Task if it is not TaskSpec-specific.
• Are the other hook methods all needed?
• Can Task._update_children be split into _update_children and _update_child_state?

[knipknap]

The TRIGGERED state has been replaced by an attribute. ee8a620

[knipknap]

A lot of the confusion in TaskSpec comes from the fact that the purpose of the some methods is not defined clearly enough. I did some preliminary work to organize this better and the plan is to implement the following:

• _update_state() and _update_state_hook(): Should be merged with _predict()/_predict_hook(). Call the resulting methods _update() and _update_hook(). _update() recursively updates the derivation tree as well as it can given the currently available information. That means it will add or remove children from the derivation tree, AND update the status of all children.
• _try_fire() methods: Renamed to _start(), those should do the work, update the status of the directly associated task spec, and initiate a call to _update() when there is reason to believe that the derivation tree may be affected. This method does NOT change the status of any other task, nor does it change the structure of the derivation tree. An exception is when the purpose of the task is to trigger or cancel a target, in which case this method may change the status of the target before calling update().
• _trigger() methods: Just like _start(), except they may also change the status of the triggered task.
• _on_complete() methods: Renamed to _confirm(). They do not change the derivation tree, and do not change the tree's status.
• _on_ready() methods: TBD. They may not change the status of the triggered task.
• _on_cancel(): TBD

[knipknap]

Some TaskSpec objects have a state even though they should not. E.g. Trigger.queued needs a place to store Workflow global data.

[ziadsawalha]

In which state should a task that has experienced an error condition be? There's no error status.

[knipknap]

In general, the control of a workflow allows for three options after a task has started:

• Allow for trying the task again, or
• allow to continue as if the task was successful, or
• do not allow to continue with the branch.

The existing states already allow for modeling this behavior, and the task implementation can decide whichever status makes the most sense after an error:

• If the task is to be tried again, go to FUTURE or WAITING,
• if the task may continue, go to COMPLETED,
• else go to CANCELLED.

It is possible that adding a specific ERROR state makes some things simpler, but I am not sure how that should affect the flow; either of the above options should still be available. Do you have a specific use case where this would have helped?

[knipknap]

Another refactoring: Some of the BPMN-layer specific code that just landed in master should move into the core object model:

Adding support for Sequence objects may be useful to the core model; the extra place for storing data and docs make sense when creating a UI editor. This may actually allow for merging the BPMN "emulation layer" with the core model: All BPMNSpecs could be converted into standard TaskSpec objects, and used interchangably with SpiffWorkflow's existing model. Similarly, the storage objects could use the standard serializer API.