New State type: TRIGGER_FAILED about prefect

[jlowin]

I'm on board with TriggerFailed subclassing Failed -- one of the reasons I was hesitant about that at first was it would then trigger subsequent tasks as if the task had failed (for example, if the subsequent task had an all_failed trigger) but your skip analogy helps me see that that is probably a good thing

[cicdw]

@jlowin Just so we're on the same page, do you agree that this test should pass and that this gets at the heart of the matter?

def test_flow_runner_with_trigger_task():
    flow = prefect.Flow()
    task1 = ErrorTask()
    task2 = SuccessTask()
    flow.add_edge(task1, task2)
    flow_state = FlowRunner(flow=flow).run(return_tasks=[task1, task2]) 
    assert isinstance(flow_state, Failed)                                        
    assert isinstance(flow_state.data[task1], Success)                           
    assert isinstance(flow_state.data[task2], Trigger_Failed)
    assert not isinstance(flow_state.data[task2], Failed)

[jlowin]

Yes, I agree. Moreover, it also isn't a SUCCESS State, either.

[cicdw]

Question: how should TriggerFailed terminal states affect the overall status of a Flow, and should it be a hard-and-fast rule or user-determined?

If we include TriggerFailed as a not-failed, not-successful Finished state, then our current flow-level state-logic renders the above Flow a Success which doesn't feel right.

I've played with having TriggerFailed take an optional is_failure parameter determining whether it should be treated as such, but these leads to all sorts of weird inconsistencies with TriggerFailed().is_failed() is True vs. isinstance(TriggerFailed(), Failed) is False.

[cicdw]

I'm starting to suspect that TriggerFailed should just be a subclass of Failed (maybe it's the Failed analogue of Skipped), and whether that results in Flow failures or not can be adjusted via #26 (i.e., if it's a failure cleanup task, then the Flow would be looking to the inner task for its state, not the TriggerFailed task).

[cicdw]

Closed with #59