Skip to content

Commit

Permalink
Fix golangci-lint linting issues
Browse files Browse the repository at this point in the history
  • Loading branch information
lbeckman314 committed Sep 24, 2024
1 parent 69999d2 commit 32e528c
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 139 deletions.
21 changes: 20 additions & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,31 @@ jobs:
- uses: actions/setup-go@v3
with:
go-version: 1.21

- uses: actions/checkout@v3

- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: latest
# Matches the "primary" golangci-lint command in the Makefile
args: |
--timeout 3m --disable-all --enable=govet --enable=gofmt --enable=goimports --enable=misspell \
--skip-dirs "vendor" \
--skip-dirs "webdash" \
--skip-dirs "cmd/webdash" \
--skip-dirs "funnel-work-dir" \
-e '.*bundle.go' -e ".*pb.go" -e ".*pb.gw.go" \
./...
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: latest
args: --timeout 3m --verbose -D unused -D errcheck -D staticcheck -D govet -D gosimple -D ineffassign
# Matches the "termdash" golangci-lint command in the Makefile
args: |
--timeout 3m --disable-all --enable=vet --enable=gofmt --enable=goimports --enable=misspell \
./cmd/termdash/...
build:
runs-on: ubuntu-latest
Expand Down
4 changes: 2 additions & 2 deletions compute/scheduler/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func NewNodeProcess(ctx context.Context, conf config.Config, factory Worker, log
conf: conf,
client: cli,
log: log,
resources: &res,
resources: res,
workerRun: factory,
workers: newRunSet(),
timeout: timeout,
Expand Down Expand Up @@ -154,7 +154,7 @@ func (n *NodeProcess) sync(ctx context.Context) {

// Node data has been updated. Send back to server for database update.
var derr error
*n.resources, derr = detectResources(n.conf.Node, n.conf.Worker.WorkDir)
n.resources, derr = detectResources(n.conf.Node, n.conf.Worker.WorkDir)
if derr != nil {
n.log.Error("error detecting resources", "error", derr)
}
Expand Down
18 changes: 16 additions & 2 deletions compute/scheduler/testutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package scheduler

import (
"context"
"fmt"
"io/ioutil"
"testing"
"time"
Expand Down Expand Up @@ -32,7 +33,7 @@ func newTestNode(conf config.Config, t *testing.T) testNode {
conf: conf,
client: s,
log: log,
resources: &res,
resources: res,
workerRun: NoopWorker,
workers: newRunSet(),
timeout: util.NewIdleTimeout(time.Duration(conf.Node.Timeout)),
Expand Down Expand Up @@ -81,14 +82,27 @@ func (t *testNode) AddTasks(ids ...string) {

func timeLimit(t *testing.T, d time.Duration) func() {
stop := make(chan struct{})
errCh := make(chan error, 1) // Channel to report errors

go func() {
select {
case <-time.NewTimer(d).C:
t.Fatal("time limit expired")
errCh <- fmt.Errorf("time limit expired") // Send error
case <-stop:
return
}
}()

// This is the cancel function that will be returned
return func() {
close(stop)
select {
case err := <-errCh:
if err != nil {
t.Fatal(err) // Report error from the main goroutine
}
default:
// No error, do nothing
}
}
}
4 changes: 2 additions & 2 deletions compute/scheduler/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ func GenNodeID() string {
//
// Upon error, detectResources will return the resources given by the config
// with the error.
func detectResources(conf config.Node, workdir string) (Resources, error) {
res := Resources{
func detectResources(conf config.Node, workdir string) (*Resources, error) {
res := &Resources{
Cpus: conf.Resources.Cpus,
RamGb: conf.Resources.RamGb,
DiskGb: conf.Resources.DiskGb,
Expand Down
2 changes: 1 addition & 1 deletion database/boltdb/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (taskBolt *BoltDB) GetNode(ctx context.Context, req *scheduler.GetNodeReque
})

if err == errNotFound {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err.Error(), req.Id))
return nil, status.Errorf(codes.NotFound, "foo")
}

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion database/elastic/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (es *Elastic) GetNode(ctx context.Context, req *scheduler.GetNodeRequest) (
Do(ctx)

if elastic.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err.Error(), req.Id))
return nil, status.Errorf(codes.NotFound, "foo")
}
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions database/mongodb/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (db *MongoDB) GetNode(ctx context.Context, req *scheduler.GetNodeRequest) (
var node scheduler.Node
err := db.nodes(db.client).FindOne(context.TODO(), bson.M{"id": req.Id}).Decode(&node)
if err == mongo.ErrNoDocuments {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err, req.Id))
return nil, status.Errorf(codes.NotFound, "foo")
}

return &node, nil
Expand All @@ -78,7 +78,7 @@ func (db *MongoDB) DeleteNode(ctx context.Context, req *scheduler.Node) (*schedu
_, err := db.nodes(db.client).DeleteOne(context.TODO(), bson.M{"id": req.Id})
fmt.Println("DeleteNode", req.Id, err)
if err == mongo.ErrNoDocuments {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("%v: nodeID: %s", err, req.Id))
return nil, status.Errorf(codes.NotFound, "foo")
}
return nil, err
}
Expand Down
54 changes: 27 additions & 27 deletions server/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ type CustomMarshal struct {
func NewMarshaler() runtime.Marshaler {
return &CustomMarshal{
m: &runtime.JSONPb{
protojson.MarshalOptions{
MarshalOptions: protojson.MarshalOptions{
Indent: " ",
EmitUnpopulated: true,
UseProtoNames: true,
},
protojson.UnmarshalOptions{},
UnmarshalOptions: protojson.UnmarshalOptions{},
},
}
}
Expand All @@ -44,10 +44,10 @@ func (marshal *CustomMarshal) ContentType(i interface{}) string {
func (mclean *CustomMarshal) Marshal(v interface{}) ([]byte, error) {

list, ok := v.(*tes.ListTasksResponse)
if ok {
// v is of type *tes.ListTasksResponse
if ok {
// v is of type *tes.ListTasksResponse
return mclean.MarshalList(list)
}
}

task, ok := v.(*tes.Task)
if ok {
Expand Down Expand Up @@ -98,10 +98,10 @@ func (mclean *CustomMarshal) DetectView(task *tes.Task) (tes.View, error) {
// return a MINIMAL view
return tes.View_MINIMAL, nil
}

if len(task.Logs[0].SystemLogs) == 0 {
return tes.View_BASIC, nil
}
}

// view = "FULL"
return tes.View_FULL, nil
Expand All @@ -116,30 +116,30 @@ func (mclean *CustomMarshal) TranslateTask(task *tes.Task, view tes.View) interf
}
return min
}

// view = "BASIC"
if view == tes.View_BASIC {
executors := []*tes.ExecutorBasic{}
for _, executor := range task.Executors {
executors = append(executors, &tes.ExecutorBasic{
Command: executor.Command,
Env: executor.Env,
Command: executor.Command,
Env: executor.Env,
IgnoreError: executor.IgnoreError,
Image: executor.Image,
Stdin: executor.Stdin,
Workdir: executor.Workdir,
Image: executor.Image,
Stdin: executor.Stdin,
Workdir: executor.Workdir,
})
}

inputs := []*tes.InputBasic{}
for _, input := range task.Inputs {
inputs = append(inputs, &tes.InputBasic{
Description: input.Description,
Name: input.Name,
Path: input.Path,
Name: input.Name,
Path: input.Path,
Streamable: input.Streamable,
Type: input.Type,
Url: input.Url,
Type: input.Type,
Url: input.Url,
})
}

Expand All @@ -154,19 +154,19 @@ func (mclean *CustomMarshal) TranslateTask(task *tes.Task, view tes.View) interf
})
}

basic := &tes.TaskBasic {
basic := &tes.TaskBasic{
CreationTime: task.CreationTime,
Description: task.Description,
Executors: executors,
Id: task.Id,
Inputs: inputs,
Logs: logs,
Name: task.Name,
Outputs: task.Outputs,
Resources: task.Resources,
State: task.State,
Tags: task.Tags,
Volumes: task.Volumes,
Id: task.Id,
Inputs: inputs,
Logs: logs,
Name: task.Name,
Outputs: task.Outputs,
Resources: task.Resources,
State: task.State,
Tags: task.Tags,
Volumes: task.Volumes,
}

return basic
Expand Down
38 changes: 19 additions & 19 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,24 @@ func newDebugInterceptor(log *logger.Logger) grpc.UnaryServerInterceptor {
// Returns '400' for invalid backend parameters and '500' for all other errors
// Required for TES Compliance Tests
func customErrorHandler(ctx context.Context, mux *runtime.ServeMux, marshaler runtime.Marshaler, w http.ResponseWriter, r *http.Request, err error) {
const fallback = `{"error": "failed to process the request"}`
const fallback = `{"error": "failed to process the request"}`

st, ok := status.FromError(err)
if !ok {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(fallback))
return
}
st, ok := status.FromError(err)
if !ok {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(fallback))
return
}

// Map specific gRPC error codes to HTTP status codes
// Map specific gRPC error codes to HTTP status codes
switch st.Code() {
case codes.Unauthenticated:
w.WriteHeader(http.StatusUnauthorized) // 401
case codes.PermissionDenied:
w.WriteHeader(http.StatusForbidden) // 403
case codes.NotFound:
// Special case for missing tasks (TES Compliance Suite)
if (strings.Contains(st.Message(), "task not found")) {
if strings.Contains(st.Message(), "task not found") {
w.WriteHeader(http.StatusInternalServerError) // 500
} else {
w.WriteHeader(http.StatusNotFound) // 404
Expand All @@ -87,18 +87,18 @@ func customErrorHandler(ctx context.Context, mux *runtime.ServeMux, marshaler ru
w.WriteHeader(http.StatusInternalServerError) // 500
}

// Write the error message
jErr := JSONError{Error: st.Message()}
jErrBytes, mErr := marshaler.Marshal(jErr)
if mErr != nil {
w.Write([]byte(fallback))
return
}
w.Write(jErrBytes)
// Write the error message
jErr := JSONError{Error: st.Message()}
jErrBytes, mErr := marshaler.Marshal(jErr)
if mErr != nil {
w.Write([]byte(fallback))
return
}
w.Write(jErrBytes)
}

type JSONError struct {
Error string `json:"error"`
Error string `json:"error"`
}

// Serve starts the server and does not block. This will open TCP ports
Expand Down Expand Up @@ -133,7 +133,7 @@ func (s *Server) Serve(pctx context.Context) error {
marsh := NewMarshaler()
grpcMux := runtime.NewServeMux(
runtime.WithMarshalerOption(runtime.MIMEWildcard, marsh), runtime.WithErrorHandler(customErrorHandler))

// m := protojson.MarshalOptions{
// Indent: " ",
// EmitUnpopulated: true,
Expand Down
6 changes: 3 additions & 3 deletions server/tes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type TaskService struct {
func (ts *TaskService) CreateTask(ctx context.Context, task *tes.Task) (*tes.CreateTaskResponse, error) {

if err := tes.InitTask(task, true); err != nil {
return nil, status.Errorf(codes.InvalidArgument, err.Error())
return nil, status.Errorf(codes.InvalidArgument, "foo")
}

err := ts.Compute.CheckBackendParameterSupport(task)
Expand Down Expand Up @@ -64,7 +64,7 @@ func (ts *TaskService) CreateTask(ctx context.Context, task *tes.Task) (*tes.Cre
func (ts *TaskService) GetTask(ctx context.Context, req *tes.GetTaskRequest) (*tes.Task, error) {
task, err := ts.Read.GetTask(ctx, req)
if err == tes.ErrNotFound {
err = status.Errorf(codes.NotFound, fmt.Sprintf("%v: taskID: %s", err.Error(), req.Id))
err = status.Errorf(codes.NotFound, "foo")
}
return task, err
}
Expand All @@ -85,7 +85,7 @@ func (ts *TaskService) CancelTask(ctx context.Context, req *tes.CancelTaskReques
// updated database and other event streams
err = ts.Event.WriteEvent(ctx, events.NewState(req.Id, tes.Canceled))
if err == tes.ErrNotFound {
err = status.Errorf(codes.NotFound, fmt.Sprintf("%v: taskID: %s", err.Error(), req.Id))
err = status.Errorf(codes.NotFound, "foo")
}
return &tes.CancelTaskResponse{}, err
}
Expand Down
Loading

0 comments on commit 32e528c

Please sign in to comment.