search

bacalhau-project / bacalhau

Compute over Data framework for public, transparent, and optionally verifiable computation
https://docs.bacalhau.org
Apache License 2.0
675 stars 87 forks source link

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

Closed frrist closed 2 months ago

frrist commented 3 months ago

The model Execution considers the following states to be terminal:

https://github.com/bacalhau-project/bacalhau/blob/17dbd8dac2fc47485b2a752f40d0d414078e6fe3/pkg/model/execution_state.go#L77-L79 https://github.com/bacalhau-project/bacalhau/blob/17dbd8dac2fc47485b2a752f40d0d414078e6fe3/pkg/model/execution_state.go#L60-L63

The model Execution considers the following states to be terminal:

https://github.com/bacalhau-project/bacalhau/blob/17dbd8dac2fc47485b2a752f40d0d414078e6fe3/pkg/models/execution.go#L45-L50

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

wdbaruni commented 3 months ago

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 commented 3 months ago

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.