model.Execution and models.Execution have different definitions of terminal state

[frrist]

The model Execution considers the following states to be terminal:

• ExecutionStateAskForBidRejected
• ExecutionStateBidRejected
• ExecutionStateCancelled
• ExecutionStateFailed
• ExecutionStateCompleted

bacalhau/pkg/model/execution_state.go

Lines 77 to 79 in 17dbd8d

	func (s ExecutionStateType) IsTerminal() bool {
	return s.IsDiscarded() || s == ExecutionStateCompleted
	}

bacalhau/pkg/model/execution_state.go

Lines 60 to 63 in 17dbd8d

	func (s ExecutionStateType) IsDiscarded() bool {
	return s == ExecutionStateAskForBidRejected || s == ExecutionStateBidRejected ||
	s == ExecutionStateCancelled || s == ExecutionStateFailed
	}

The model Execution considers the following states to be terminal:

• ExecutionStateBidRejected
• ExecutionStateCompleted
• ExecutionStateFailed
• ExecutionStateCancelled

bacalhau/pkg/models/execution.go

Lines 45 to 50 in 17dbd8d

	func (s ExecutionStateType) IsTermainl() bool {
	return s == ExecutionStateBidRejected ||
	s == ExecutionStateCompleted ||
	s == ExecutionStateFailed ||
	s == ExecutionStateCancelled
	}

The different being ExecutionStateAskForBidRejected. @wdbaruni is this a bug?

[wdbaruni]

Looks like a bug. Actually we've stopped using ExecutionStateType.IsTerminal() in favour of Execution.IsTerminalState(), but it is still being used in one place when printing cli progress

[frrist]

Got it, I have fixed this here: https://github.com/bacalhau-project/bacalhau/pull/4147/files#diff-b63449354c9d58981e007903938dbaac6119844a3eaec272e500e838cb28a0b0R50. Will close this issue out when #4147 merges as I noticed this bug when updating some of our tests to the new models.