Skip to content

Commit

Permalink
Working on code formatting and lintint
Browse files Browse the repository at this point in the history
  • Loading branch information
kellrott authored and lbeckman314 committed Oct 28, 2023
1 parent 748040a commit 518a53c
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 48 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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" \
Expand Down
3 changes: 2 additions & 1 deletion compute/scheduler/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion database/boltdb/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
17 changes: 11 additions & 6 deletions logger/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
22 changes: 17 additions & 5 deletions tests/core/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'`)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
9 changes: 8 additions & 1 deletion tests/pubsub/pubsub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}()

Expand All @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions tests/storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 19 additions & 19 deletions util/idle_timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions util/openapi2proto/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Field struct {
Name string
Type string
Repeated bool
Id int
ID int
Source *openapi3.SchemaRef
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions worker/file_mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down

0 comments on commit 518a53c

Please sign in to comment.