Manager: add timeout when fetching job
Add a timeout when fetching a job from the persistence layers. It's my intention to add more timeouts, so this also introduces some code to make it easier to test that a context has a deadline set.
This commit is contained in:
parent
c16c1f4b15
commit
9bda21648e
@ -18,10 +18,8 @@ import (
|
||||
// to the HTTP client if it cannot. Returns `nil` in the latter case, and the
|
||||
// error returned can then be returned from the Echo handler function.
|
||||
func (f *Flamenco) fetchJob(e echo.Context, logger zerolog.Logger, jobID string) (*persistence.Job, error) {
|
||||
// TODO: use a timeout for fetching jobs.
|
||||
// ctx, cancel := context.WithTimeout(e.Request().Context(), fetchJobTimeout)
|
||||
// defer cancel()
|
||||
ctx := e.Request().Context()
|
||||
ctx, cancel := context.WithTimeout(e.Request().Context(), fetchJobTimeout)
|
||||
defer cancel()
|
||||
|
||||
if !uuid.IsValid(jobID) {
|
||||
logger.Debug().Msg("invalid job ID received")
|
||||
|
@ -14,6 +14,7 @@ import (
|
||||
"git.blender.org/flamenco/internal/manager/last_rendered"
|
||||
"git.blender.org/flamenco/internal/manager/persistence"
|
||||
"git.blender.org/flamenco/pkg/api"
|
||||
"git.blender.org/flamenco/pkg/moremock"
|
||||
"github.com/golang/mock/gomock"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
@ -408,7 +409,9 @@ func TestSetJobPrio_nonexistentJob(t *testing.T) {
|
||||
jobID := "18a9b096-d77e-438c-9be2-74397038298b"
|
||||
prioUpdate := api.JobPriorityChange{Priority: 47}
|
||||
|
||||
mf.persistence.EXPECT().FetchJob(gomock.Any(), jobID).Return(nil, persistence.ErrJobNotFound)
|
||||
mf.persistence.EXPECT().
|
||||
FetchJob(moremock.ContextWithDeadline(), jobID).
|
||||
Return(nil, persistence.ErrJobNotFound)
|
||||
|
||||
// Do the call.
|
||||
echoCtx := mf.prepareMockedJSONRequest(prioUpdate)
|
||||
@ -438,7 +441,7 @@ func TestSetJobPrio(t *testing.T) {
|
||||
|
||||
// Set up expectations.
|
||||
ctx := echoCtx.Request().Context()
|
||||
mf.persistence.EXPECT().FetchJob(ctx, jobID).Return(&dbJob, nil).AnyTimes()
|
||||
mf.persistence.EXPECT().FetchJob(moremock.ContextWithDeadline(), jobID).Return(&dbJob, nil).AnyTimes()
|
||||
jobWithNewPrio := dbJob
|
||||
jobWithNewPrio.Priority = 47
|
||||
mf.persistence.EXPECT().SaveJobPriority(gomock.Not(ctx), &jobWithNewPrio)
|
||||
@ -482,7 +485,7 @@ func TestSetJobStatusFailedToRequeueing(t *testing.T) {
|
||||
// Set up expectations.
|
||||
echoCtx := mf.prepareMockedJSONRequest(statusUpdate)
|
||||
ctx := echoCtx.Request().Context()
|
||||
mf.persistence.EXPECT().FetchJob(ctx, jobID).Return(&dbJob, nil)
|
||||
mf.persistence.EXPECT().FetchJob(moremock.ContextWithDeadline(), jobID).Return(&dbJob, nil)
|
||||
mf.stateMachine.EXPECT().JobStatusChange(ctx, &dbJob, statusUpdate.Status, "someone pushed a button")
|
||||
mf.persistence.EXPECT().ClearFailureListOfJob(ctx, &dbJob)
|
||||
mf.persistence.EXPECT().ClearJobBlocklist(ctx, &dbJob)
|
||||
|
9
internal/manager/api_impl/timeouts.go
Normal file
9
internal/manager/api_impl/timeouts.go
Normal file
@ -0,0 +1,9 @@
|
||||
package api_impl
|
||||
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
import "time"
|
||||
|
||||
const (
|
||||
fetchJobTimeout = 5 * time.Second
|
||||
)
|
36
pkg/moremock/context.go
Normal file
36
pkg/moremock/context.go
Normal file
@ -0,0 +1,36 @@
|
||||
package moremock
|
||||
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
import (
|
||||
"context"
|
||||
"time"
|
||||
|
||||
"github.com/golang/mock/gomock"
|
||||
)
|
||||
|
||||
// ContextWithDeadline returns a gomock matcher to match a context.Context()
|
||||
// with a deadline in the future.
|
||||
func ContextWithDeadline() gomock.Matcher { return ctxWithDeadlineMatcher{} }
|
||||
|
||||
type ctxWithDeadlineMatcher struct{}
|
||||
|
||||
// Matches returns whether x is a match.
|
||||
func (m ctxWithDeadlineMatcher) Matches(x interface{}) bool {
|
||||
ctx, ok := x.(context.Context)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
deadline, ok := ctx.Deadline()
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
return time.Now().Before(deadline)
|
||||
}
|
||||
|
||||
// String describes what the matcher matches.
|
||||
func (m ctxWithDeadlineMatcher) String() string {
|
||||
return "is context with deadline in the future"
|
||||
}
|
46
pkg/moremock/context_test.go
Normal file
46
pkg/moremock/context_test.go
Normal file
@ -0,0 +1,46 @@
|
||||
package moremock
|
||||
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestCtxMatcher(t *testing.T) {
|
||||
m := ctxWithDeadlineMatcher{}
|
||||
|
||||
// Non-context types -> No match.
|
||||
assert.False(t, m.Matches(nil))
|
||||
assert.False(t, m.Matches("something else"))
|
||||
var nilValuedInterface context.Context
|
||||
assert.False(t, m.Matches(nilValuedInterface))
|
||||
|
||||
// Context without deadlines -> No match.
|
||||
assert.False(t, m.Matches(context.Background()))
|
||||
assert.False(t, m.Matches(context.TODO()))
|
||||
|
||||
// Deadline in the past -> No match.
|
||||
{
|
||||
past, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second))
|
||||
defer cancel()
|
||||
assert.False(t, m.Matches(past))
|
||||
}
|
||||
|
||||
// Deadline in the future -> Match.
|
||||
{
|
||||
future, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Second))
|
||||
defer cancel()
|
||||
assert.True(t, m.Matches(future))
|
||||
}
|
||||
|
||||
// Timeout in the future -> Match.
|
||||
{
|
||||
future, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||
defer cancel()
|
||||
assert.True(t, m.Matches(future))
|
||||
}
|
||||
}
|
4
pkg/moremock/moremock.go
Normal file
4
pkg/moremock/moremock.go
Normal file
@ -0,0 +1,4 @@
|
||||
// Package moremock contains extra matchers for the gomock package.
|
||||
//
|
||||
// For info on gomock, see https://pkg.go.dev/github.com/golang/mock/gomock
|
||||
package moremock
|
Loading…
Reference in New Issue
Block a user