From 518a53cb7af9bdeb874879ce16e2b995f28a0ee3 Mon Sep 17 00:00:00 2001 From: Kyle Ellrott Date: Thu, 17 Nov 2022 13:22:47 -0800 Subject: [PATCH] Working on code formatting and lintint --- Makefile | 4 ++-- compute/scheduler/node.go | 3 ++- database/boltdb/new.go | 3 ++- go.mod | 1 + go.sum | 4 ++++ logger/log.go | 17 ++++++++++------ tests/core/basic_test.go | 22 +++++++++++++++----- tests/pubsub/pubsub_test.go | 9 ++++++++- tests/storage/storage_test.go | 12 +++++------ util/idle_timeout.go | 38 +++++++++++++++++------------------ util/openapi2proto/main.go | 10 ++++----- worker/file_mapper.go | 4 ++-- 12 files changed, 79 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index 786ccf446..80af922f6 100644 --- a/Makefile +++ b/Makefile @@ -82,12 +82,12 @@ tidy: @find . \( -path ./vendor -o -path ./webdash/node_modules -o -path ./venv -o -path ./.git \) -prune -o -type f -print | grep -v "\.pb\." | grep -v "web.go" | grep -E '.*\.go$$' | xargs gofmt -w -s lint-depends: - go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.22.2 + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1 go install golang.org/x/tools/cmd/goimports # Run code style and other checks lint: - @golangci-lint run --timeout 3m --disable-all --enable=vet --enable=golint --enable=gofmt --enable=goimports --enable=misspell \ + @golangci-lint run --timeout 3m --disable-all --enable=golint --enable=gofmt --enable=goimports --enable=misspell \ --skip-dirs "vendor" \ --skip-dirs "webdash" \ --skip-dirs "cmd/webdash" \ diff --git a/compute/scheduler/node.go b/compute/scheduler/node.go index bd2ec874c..da964c0ad 100644 --- a/compute/scheduler/node.go +++ b/compute/scheduler/node.go @@ -125,7 +125,8 @@ func (n *NodeProcess) checkConnection(ctx context.Context) { // handles signals from the server (new task, cancel task, etc), reports resources, etc. // // TODO Sync should probably use a channel to sync data access. -// Probably only a problem for test code, where Sync is called directly. +// +// Probably only a problem for test code, where Sync is called directly. func (n *NodeProcess) sync(ctx context.Context) { var r *Node var err error diff --git a/database/boltdb/new.go b/database/boltdb/new.go index 64c36cc2f..e68f84ff9 100644 --- a/database/boltdb/new.go +++ b/database/boltdb/new.go @@ -40,7 +40,8 @@ var ExecutorStderr = []byte("executor-stderr") var Nodes = []byte("nodes") // SysLogs defeines the name of a bucket with maps -// task ID -> tes.TaskLog.SystemLogs +// +// task ID -> tes.TaskLog.SystemLogs var SysLogs = []byte("system-logs") // BoltDB provides handlers for gRPC endpoints. diff --git a/go.mod b/go.mod index 80ea00f72..68b87f6ec 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/golang/protobuf v1.5.2 github.com/grpc-ecosystem/go-grpc-middleware v1.2.0 github.com/grpc-ecosystem/grpc-gateway/v2 v2.5.0 + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/imdario/mergo v0.3.8 github.com/jlaffaye/ftp v0.0.0-20191218041957-e1b8fdd0dcc3 github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af // indirect diff --git a/go.sum b/go.sum index 185f4fb3c..dcac0a980 100644 --- a/go.sum +++ b/go.sum @@ -280,6 +280,10 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.2.0 h1:0IKlLyQ3Hs9nDaiK5cSHAGmcQ github.com/grpc-ecosystem/go-grpc-middleware v1.2.0/go.mod h1:mJzapYve32yjrKlk9GbyCZHuPgZsrbyIbyKhSzOpg6s= github.com/grpc-ecosystem/grpc-gateway/v2 v2.5.0 h1:ajue7SzQMywqRjg2fK7dcpc0QhFGpTR2plWfV4EZWR4= github.com/grpc-ecosystem/grpc-gateway/v2 v2.5.0/go.mod h1:r1hZAcvfFXuYmcKyCJI9wlyOPIZUJl6FCB8Cpca/NLE= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE= github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= diff --git a/logger/log.go b/logger/log.go index 361ce894c..497385f5c 100644 --- a/logger/log.go +++ b/logger/log.go @@ -80,7 +80,8 @@ func (l *Logger) Discard() { // Debug logs a debug message. // // After the first argument, arguments are key-value pairs which are written as structured logs. -// log.Debug("Some message here", "key1", value1, "key2", value2) +// +// log.Debug("Some message here", "key1", value1, "key2", value2) func (l *Logger) Debug(msg string, args ...interface{}) { if l == nil { return @@ -93,7 +94,8 @@ func (l *Logger) Debug(msg string, args ...interface{}) { // Info logs an info message // // After the first argument, arguments are key-value pairs which are written as structured logs. -// log.Info("Some message here", "key1", value1, "key2", value2) +// +// log.Info("Some message here", "key1", value1, "key2", value2) func (l *Logger) Info(msg string, args ...interface{}) { if l == nil { return @@ -106,11 +108,13 @@ func (l *Logger) Info(msg string, args ...interface{}) { // Error logs an error message // // After the first argument, arguments are key-value pairs which are written as structured logs. -// log.Error("Some message here", "key1", value1, "key2", value2) +// +// log.Error("Some message here", "key1", value1, "key2", value2) // // Error has a two-argument version that can be used as a shortcut. -// err := startServer() -// log.Error("Couldn't start server", err) +// +// err := startServer() +// log.Error("Couldn't start server", err) func (l *Logger) Error(msg string, args ...interface{}) { if l == nil { return @@ -136,7 +140,8 @@ func (l *Logger) Error(msg string, args ...interface{}) { // Warn logs an warning message // // After the first argument, arguments are key-value pairs which are written as structured logs. -// log.Info("Some message here", "key1", value1, "key2", value2) +// +// log.Info("Some message here", "key1", value1, "key2", value2) func (l *Logger) Warn(msg string, args ...interface{}) { if l == nil { return diff --git a/tests/core/basic_test.go b/tests/core/basic_test.go index a806b4e82..6d5ed83ee 100644 --- a/tests/core/basic_test.go +++ b/tests/core/basic_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/hashicorp/go-multierror" workerCmd "github.com/ohsu-comp-bio/funnel/cmd/worker" "github.com/ohsu-comp-bio/funnel/events" "github.com/ohsu-comp-bio/funnel/tes" @@ -280,9 +281,10 @@ func TestGetTaskView(t *testing.T) { } // TODO this is a bit hacky for now because we're reusing the same -// server + DB for all the e2e tests, so ListTasks gets the -// results of all of those. It works for the moment, but -// should probably run against a clean environment. +// +// server + DB for all the e2e tests, so ListTasks gets the +// results of all of those. It works for the moment, but +// should probably run against a clean environment. func TestListTaskView(t *testing.T) { tests.SetLogOutput(log, t) var tasks []*tes.Task @@ -1117,6 +1119,8 @@ func TestConcurrentStateUpdate(t *testing.T) { f := tests.NewFunnel(c) f.StartServer() + var result *multierror.Error + ids := []string{} for i := 0; i < 10; i++ { id := f.Run(`--sh 'echo hello'`) @@ -1126,12 +1130,14 @@ func TestConcurrentStateUpdate(t *testing.T) { opts := &workerCmd.Options{TaskID: id} w, err := workerCmd.NewWorker(ctx, c, log, opts) if err != nil { - t.Fatal(err) + result = multierror.Append(result, err) + return } log.Info("writing state initializing event", "taskID", id) err = w.EventWriter.WriteEvent(ctx, events.NewState(id, tes.Initializing)) if err != nil { + result = multierror.Append(result, err) log.Error("error writing event", err) } }(id) @@ -1140,17 +1146,23 @@ func TestConcurrentStateUpdate(t *testing.T) { opts := &workerCmd.Options{TaskID: id} w, err := workerCmd.NewWorker(ctx, c, log, opts) if err != nil { - t.Fatal(err) + result = multierror.Append(result, err) + return } log.Info("writing state canceled event", "taskID", id) err = w.EventWriter.WriteEvent(ctx, events.NewState(id, tes.Canceled)) if err != nil { + result = multierror.Append(result, err) log.Error("error writing event", "error", err, "taskID", id) } }(id) } + if result != nil { + t.Error(result) + } + for _, i := range ids { log.Info("waiting for task", "taskID", i) task := f.Wait(i) diff --git a/tests/pubsub/pubsub_test.go b/tests/pubsub/pubsub_test.go index b2543e266..e2cd33530 100644 --- a/tests/pubsub/pubsub_test.go +++ b/tests/pubsub/pubsub_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/hashicorp/go-multierror" workerCmd "github.com/ohsu-comp-bio/funnel/cmd/worker" "github.com/ohsu-comp-bio/funnel/config" "github.com/ohsu-comp-bio/funnel/events" @@ -50,12 +51,14 @@ func TestPubSubWorkerRun(t *testing.T) { task := &tes.Task{} b := events.TaskBuilder{Task: task} + var result *multierror.Error + // Read events from pubsub, write into task builder. subname := "test-pubsub-" + tests.RandomString(10) go func() { err := events.ReadPubSub(ctx, conf.PubSub, subname, b) if err != nil { - t.Fatal(err) + result = multierror.Append(result, err) } }() @@ -70,6 +73,10 @@ func TestPubSubWorkerRun(t *testing.T) { fun.Wait(id) time.Sleep(time.Second) + if result != nil { + t.Fatal(result) + } + // Check the task (built from a stream of kafka events). if task.State != tes.State_COMPLETE { t.Fatal("unexpected state", task.State) diff --git a/tests/storage/storage_test.go b/tests/storage/storage_test.go index a658ef8b2..aa62360d8 100644 --- a/tests/storage/storage_test.go +++ b/tests/storage/storage_test.go @@ -95,13 +95,13 @@ func TestBrokenSymlinkInput(t *testing.T) { } /* - Test the case where a container creates a symlink in an output path. - From the view of the host system where Funnel is running, this creates - a broken link, because the source of the symlink is a path relative - to the container filesystem. +Test the case where a container creates a symlink in an output path. +From the view of the host system where Funnel is running, this creates +a broken link, because the source of the symlink is a path relative +to the container filesystem. - Funnel can fix some of these cases using volume definitions, which - is being tested here. +Funnel can fix some of these cases using volume definitions, which +is being tested here. */ func TestSymlinkOutput(t *testing.T) { tests.SetLogOutput(log, t) diff --git a/util/idle_timeout.go b/util/idle_timeout.go index 9c41d3b42..cca59101a 100644 --- a/util/idle_timeout.go +++ b/util/idle_timeout.go @@ -6,18 +6,18 @@ import "time" // Start() and Stop() are used to control the timer, and Done() is used to // detect when the timeout has been reached. // -// in := make(chan int) -// requestInput(in) -// t := IdleTimeoutAfter(time.Second * 10) -// for { -// select { -// case <-t.Done(): -// // ... code to respond to timeout -// case <-in: -// // Reset the timeout. -// t.Start() -// } -// } +// in := make(chan int) +// requestInput(in) +// t := IdleTimeoutAfter(time.Second * 10) +// for { +// select { +// case <-t.Done(): +// // ... code to respond to timeout +// case <-in: +// // Reset the timeout. +// t.Start() +// } +// } type IdleTimeout interface { Done() <-chan time.Time Start() @@ -51,13 +51,13 @@ type timerTimeout struct { // Done returns a channel which can be used to wait for the timeout. // -// t := IdleTimeoutAfter(time.Second * 10) -// for { -// select { -// case <-t.Done(): -// // ... code to respond to timeout -// } -// } +// t := IdleTimeoutAfter(time.Second * 10) +// for { +// select { +// case <-t.Done(): +// // ... code to respond to timeout +// } +// } func (t *timerTimeout) Done() <-chan time.Time { if t.timer != nil { return t.timer.C diff --git a/util/openapi2proto/main.go b/util/openapi2proto/main.go index 1f511eb90..c52787081 100644 --- a/util/openapi2proto/main.go +++ b/util/openapi2proto/main.go @@ -16,7 +16,7 @@ type Field struct { Name string Type string Repeated bool - Id int + ID int Source *openapi3.SchemaRef } @@ -98,7 +98,7 @@ func getFields(schema *openapi3.SchemaRef) []Field { } sort.SliceStable(fields, func(i, j int) bool { return fields[i].Name < fields[j].Name }) for i := range fields { - fields[i].Id = i + 1 + fields[i].ID = i + 1 } return fields } @@ -133,7 +133,7 @@ func parseMessageSchema(name string, schema *openapi3.SchemaRef) (Message, error //fmt.Printf("Fields: %#vs\n", fields) sort.SliceStable(fields, func(i, j int) bool { return fields[i].Name < fields[j].Name }) for i := range fields { - fields[i].Id = i + 1 + fields[i].ID = i + 1 } m := Message{Name: name, Fields: fields} return m, nil @@ -227,7 +227,7 @@ func main() { reqFields = append(reqFields, Field{Name: param.Value.Name, Type: t, Repeated: r}) } for i := range reqFields { - reqFields[i].Id = i + 1 + reqFields[i].ID = i + 1 } m := Message{Name: req.Get.OperationID + "Request", Fields: reqFields} messages = append(messages, m) @@ -241,7 +241,7 @@ func main() { reqFields = append(reqFields, Field{Name: param.Value.Name, Type: t, Repeated: r}) } for i := range reqFields { - reqFields[i].Id = i + 1 + reqFields[i].ID = i + 1 } if req.Post.RequestBody != nil { //if not a reference to schema, build it diff --git a/worker/file_mapper.go b/worker/file_mapper.go index 2db997c2b..4219c724d 100644 --- a/worker/file_mapper.go +++ b/worker/file_mapper.go @@ -151,7 +151,7 @@ func (mapper *FileMapper) HostPath(src string) (string, error) { // OpenHostFile opens a file on the host file system at a mapped path. // "src" is an unmapped path. This function will handle mapping the path. // -// This function calls os.Open +// # This function calls os.Open // // If the path can't be mapped or the file can't be opened, an error is returned. func (mapper *FileMapper) OpenHostFile(src string) (*os.File, error) { @@ -169,7 +169,7 @@ func (mapper *FileMapper) OpenHostFile(src string) (*os.File, error) { // CreateHostFile creates a file on the host file system at a mapped path. // "src" is an unmapped path. This function will handle mapping the path. // -// This function calls os.Create +// # This function calls os.Create // // If the path can't be mapped or the file can't be created, an error is returned. func (mapper *FileMapper) CreateHostFile(src string) (*os.File, error) {