diff --git a/internal/manager/api_impl/workers.go b/internal/manager/api_impl/workers.go index f5623db6..1fbda706 100644 --- a/internal/manager/api_impl/workers.go +++ b/internal/manager/api_impl/workers.go @@ -373,7 +373,14 @@ func (f *Flamenco) doTaskUpdate( logger.Panic().Msg("dbTask.Job is nil, unable to continue") } - var dbErr error + var dbErrActivity, dbErrStatus error + + if update.Activity != nil { + dbTask.Activity = *update.Activity + // The state machine will also save the task, including its activity, but + // relying on that here would create strong cohesion. + dbErrActivity = f.persist.SaveTaskActivity(ctx, dbTask) + } if update.TaskStatus != nil { oldTaskStatus := dbTask.Status @@ -383,16 +390,11 @@ func (f *Flamenco) doTaskUpdate( Str("newTaskStatus", string(*update.TaskStatus)). Str("oldTaskStatus", string(oldTaskStatus)). Msg("error changing task status") - dbErr = fmt.Errorf("changing status of task %s to %q: %w", + dbErrStatus = fmt.Errorf("changing status of task %s to %q: %w", dbTask.UUID, *update.TaskStatus, err) } } - if update.Activity != nil { - dbTask.Activity = *update.Activity - dbErr = f.persist.SaveTaskActivity(ctx, dbTask) - } - if update.Log != nil { // Errors writing the log to file should be logged in our own logging // system, but shouldn't abort the render. As such, `err` is not returned to @@ -403,7 +405,12 @@ func (f *Flamenco) doTaskUpdate( } } - return dbErr + // Any error updating the status is more important than an error updating the + // activity. + if dbErrStatus != nil { + return dbErrStatus + } + return dbErrActivity } func (f *Flamenco) MayWorkerRun(e echo.Context, taskID string) error { diff --git a/internal/manager/api_impl/workers_test.go b/internal/manager/api_impl/workers_test.go index 234fd28a..ddf92df6 100644 --- a/internal/manager/api_impl/workers_test.go +++ b/internal/manager/api_impl/workers_test.go @@ -198,8 +198,8 @@ func TestTaskUpdate(t *testing.T) { assert.NoError(t, err) assert.Equal(t, mockTask.UUID, statusChangedtask.UUID) assert.Equal(t, mockTask.UUID, actUpdatedTask.UUID) - assert.Equal(t, "pre-update activity", statusChangedtask.Activity) // the 'save' should come from the change in status. - assert.Equal(t, "testing", actUpdatedTask.Activity) // the activity should be saved separately. + assert.Equal(t, "testing", statusChangedtask.Activity) + assert.Equal(t, "testing", actUpdatedTask.Activity) } func TestMayWorkerRun(t *testing.T) { diff --git a/internal/manager/webupdates/job_updates.go b/internal/manager/webupdates/job_updates.go index c7dd5bd3..7a060f2d 100644 --- a/internal/manager/webupdates/job_updates.go +++ b/internal/manager/webupdates/job_updates.go @@ -32,14 +32,14 @@ func NewJobUpdate(job *persistence.Job) api.JobUpdate { // Assumes task.Job is not nil. func NewTaskUpdate(task *persistence.Task) api.SocketIOTaskUpdate { taskUpdate := api.SocketIOTaskUpdate{ - Id: task.UUID, - JobId: task.Job.UUID, - Name: task.Name, - Updated: task.UpdatedAt, - Status: task.Status, + Id: task.UUID, + JobId: task.Job.UUID, + Name: task.Name, + Updated: task.UpdatedAt, + Status: task.Status, + Activity: task.Activity, } return taskUpdate - } // BroadcastJobUpdate sends the job update to clients.