studio/flamenco
studio
/
flamenco
12
27
Fork 32
Code Issues 60 Pull Requests 7 Releases Wiki Activity

Fix #99549: Remember Previous Status #104217

Merged
Sybren A. Stüvel merged 16 commits from Evelinealy/flamenco:web-api-upgrade into main 2023-06-02 22:50:10 +02:00
Conversation 7 Commits 16 Files Changed 3 +23 -8
3 changed files with 23 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
.vscode/settings.json vendored
View File

@ -13,9 +13,10 @@
},
"[yaml]": {
"editor.autoIndent": "keep",
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.defaultFormatter": "esbenp.prettier-vscode"
Sybren A. Stüvel commented 2023-06-02 10:55:54 +02:00
Review
Copy Link

Such IDE configuration changes shouldn't be in the same PR as functional changes to Flamenco itself.

This to me is showing that we shouldn't have .vscode/settings.json in the repository at all; it's too fragile. It was fine when I was the only one working on the Go code, but it's clear that this doesn't work when we both are.

To keep things simple, it's ok for now to keep these changes in here. After this PR is merged, I'll remove the file entirely and add it to .gitignore.

Such IDE configuration changes shouldn't be in the same PR as functional changes to Flamenco itself. This to me is showing that we shouldn't have `.vscode/settings.json` in the repository at all; it's too fragile. It was fine when I was the only one working on the Go code, but it's clear that this doesn't work when we both are. To keep things simple, it's ok for now to keep these changes in here. After this PR is merged, I'll remove the file entirely and add it to `.gitignore`.
},
"[vue]": {
"editor.defaultFormatter": "Vue.volar",
"editor.defaultFormatter": "Vue.volar"
},
"editor.formatOnSave": true
}

5
internal/manager/api_impl/workers.go
View File

@ -163,6 +163,11 @@ func (f *Flamenco) SignOff(e echo.Context) error {
w.StatusChangeClear()
}
// Remember the previous status if an initial status exists
if w.StatusRequested == "" {
w.StatusChangeRequest(prevStatus, false)
}
// Pass a generic background context, as these changes should be stored even
// when the HTTP connection is aborted.
bgCtx, bgCtxCancel := bgContext()

21
internal/manager/api_impl/workers_test.go
View File

@ -244,6 +244,10 @@ func TestWorkerSignoffTaskRequeue(t *testing.T) {
Name: worker.Name,
PreviousStatus: &prevStatus,
Status: api.WorkerStatusOffline,
StatusChange: &api.WorkerStatusChangeRequest{
IsLazy: false,
Status: api.WorkerStatusAwake,
},
Updated: worker.UpdatedAt,
Version: worker.Software,
})
@ -255,11 +259,11 @@ func TestWorkerSignoffTaskRequeue(t *testing.T) {
assert.Equal(t, http.StatusNoContent, resp.StatusCode)
}
func TestWorkerSignoffStatusChangeRequest(t *testing.T) {
func TestWorkerRememberPreviousStatus(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mf := newMockedFlamenco(mockCtrl)
worker := testWorker()
worker.Status = api.WorkerStatusAwake
worker.StatusChangeRequest(api.WorkerStatusOffline, true)
@ -269,25 +273,30 @@ func TestWorkerSignoffStatusChangeRequest(t *testing.T) {
Name: worker.Name,
PreviousStatus: ptr(api.WorkerStatusAwake),
Status: api.WorkerStatusOffline,
StatusChange: &api.WorkerStatusChangeRequest{
IsLazy: false,
Status: api.WorkerStatusAwake,
},
Updated: worker.UpdatedAt,
Version: worker.Software,
})
// Expect the Worker to be saved with the status change removed.
savedWorker := worker
savedWorker.Status = api.WorkerStatusOffline
savedWorker.StatusChangeClear()
savedWorker.StatusRequested = api.WorkerStatusAwake
savedWorker.LazyStatusRequest = false
mf.persistence.EXPECT().SaveWorkerStatus(gomock.Any(), &savedWorker).Return(nil)
Sybren A. Stüvel commented 2023-06-02 10:52:59 +02:00
Outdated
Review
Copy Link

I can imagine this works, as it removes the entire check against savedWorker and accepts any value instead. That's a bit too weak for a unit test though.

I can imagine this works, as it removes the entire check against `savedWorker` and accepts any value instead. That's a bit too weak for a unit test though.
mf.stateMachine.EXPECT().RequeueActiveTasksOfWorker(gomock.Any(), &worker, "worker signed off").Return(nil)
mf.persistence.EXPECT().WorkerSeen(gomock.Any(), &worker)
// Perform the request
echo := mf.prepareMockedRequest(nil)
requestWorkerStore(echo, &worker)
err := mf.flamenco.SignOff(echo)
assert.NoError(t, err)
assertResponseNoContent(t, echo)
assert.Equal(t, api.WorkerStatusAwake, worker.StatusRequested)
assert.Equal(t, false, worker.LazyStatusRequest)
}
func TestWorkerState(t *testing.T) {