flow_job does not prevent multiple execution

[martinhill]

In version 0.10.x, flow_job could only execute if task.status == STATUS.SCHEDULED. In 0.12, it will execute activation.restart(), which is allowed if task.status == STATUS.STARTED.

As a result of this change, first execution changes task status to STARTED and execute task code. During job execution, task may be called again, but when activation.done() and .activate_next() are called, TransitionNotAllowed exception is raised due to all_leading_cancelled condition. Transaction is rolled back and leaves task.status == STATUS.STARTED

Even though viewflow schedules only one job with celery, mis-configured transport can cause jobs to be repeated. For instance, when using Redis broker with job ETA time longer than visibility_timeout duration.

[martinhill]

Looking at the code changes again from #154, I think I understand the reason the restart transition was added. However, there is a problem with allowing the task to retry without a state transition from STARTED back to SCHEDULED. As currently implemented, flow_job cannot distinguish between a task being retried and another concurrent execution.

Just to suggest some ideas, changing the status to SCHEDULED could be done by handling the celery.exceptions.Retry exception in flow_job, but this wouldn't work when executed eagerly, and introduce celery-specific code which might be a problem. Another way might be to implement a custom Task subclass and implement Task.on_retry to update the task status.

A quick fix might be to simply remove STATUS.STARTED from restart() source list, but I really don't know what use case it is required to support.

[willseward]

I certainly see the problem here. I think implementing a Task.on_retry is the cleanest solution.