Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix hardware not found response #365

Merged
merged 4 commits into from
Nov 19, 2020
Merged

Fix hardware not found response #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce52ece
converge if statements in TestGetTemplate
kqdeng Nov 13, 2020
7473ee9
fix: get returns sql.ErrNoRows when hardware not found to avoid 'Unkn…
kqdeng Nov 12, 2020
8f0e53c
fix error message format
kqdeng Nov 18, 2020
99800d5
clean up some error messages
kqdeng Nov 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions db/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,11 @@ func get(ctx context.Context, db *sql.DB, query string, args ...interface{}) (st

buf := []byte{}
err := row.Scan(&buf)
if err == nil {
return string(buf), nil
}

if err != sql.ErrNoRows {
err = errors.Wrap(err, "SELECT")
if err != nil {
logger.Error(err)
} else {
err = nil
return "", errors.Wrap(err, "SELECT")
}

return "", err
return string(buf), nil
}

// buildGetCondition builds a where condition string in the format "column_name = 'field_value' AND"
Expand Down
2 changes: 1 addition & 1 deletion db/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (d TinkDB) CreateTemplate(ctx context.Context, name string, data string, id
func (d TinkDB) GetTemplate(ctx context.Context, fields map[string]string) (string, string, string, error) {
getCondition, err := buildGetCondition(fields)
if err != nil {
return "", "", "", errors.Wrap(err, "failed to build get condition")
return "", "", "", errors.Wrap(err, "failed to get template")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should actually be errors.WithMessage not wrap but we don't need to change that now. Using errors.Wrap on an already Wrapped error will show up with a new stack trace for each Wrap which isn't really what we want. WithMessage just adds more messages to the original stack trace.

}

query := `
Expand Down
26 changes: 17 additions & 9 deletions db/workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ func (d TinkDB) CreateWorkflow(ctx context.Context, wf Workflow, data string, id

err = insertActionList(ctx, d.instance, data, id, tx)
if err != nil {
return errors.Wrap(err, "Failed to insert in workflow_state")
return errors.Wrap(err, "failed to create workflow")

}
err = insertInWorkflow(ctx, d.instance, wf, tx)
if err != nil {
return errors.Wrap(err, "Failed to workflow")
return errors.Wrap(err, "failed to create workflow")

}
err = tx.Commit()
Expand Down Expand Up @@ -109,9 +109,7 @@ func insertActionList(ctx context.Context, db *sql.DB, yamlData string, id uuid.

workerID, err := getWorkerID(ctx, db, task.WorkerAddr)
if err != nil {
return err
} else if workerID == "" {
return fmt.Errorf("hardware mentioned with reference %s not found", task.WorkerAddr)
return errors.WithMessage(err, "unable to insert into action list")
}
workerUID, err := uuid.Parse(workerID)
if err != nil {
Expand Down Expand Up @@ -689,7 +687,11 @@ func getWorkerIDbyMac(ctx context.Context, db *sql.DB, mac string) (string, erro
data @> $1
`

return get(ctx, db, query, arg)
id, err := get(ctx, db, query, arg)
if errors.Cause(err) == sql.ErrNoRows {
err = errors.WithMessage(errors.New(mac), "mac")
}
return id, err
}

func getWorkerIDbyIP(ctx context.Context, db *sql.DB, ip string) (string, error) {
Expand Down Expand Up @@ -733,7 +735,11 @@ func getWorkerIDbyIP(ctx context.Context, db *sql.DB, ip string) (string, error)
)
`

return get(ctx, db, query, instance, hardwareOrManagement)
id, err := get(ctx, db, query, instance, hardwareOrManagement)
if errors.Cause(err) == sql.ErrNoRows {
err = errors.WithMessage(errors.New(ip), "ip")
}
return id, err
}

func getWorkerID(ctx context.Context, db *sql.DB, addr string) (string, error) {
Expand All @@ -743,10 +749,12 @@ func getWorkerID(ctx context.Context, db *sql.DB, addr string) (string, error) {
if ip == nil || ip.To4() == nil {
return "", fmt.Errorf("invalid worker address: %s", addr)
}
return getWorkerIDbyIP(ctx, db, addr)
id, err := getWorkerIDbyIP(ctx, db, addr)
return id, errors.WithMessage(err, "no worker found")

}
return getWorkerIDbyMac(ctx, db, addr)
id, err := getWorkerIDbyMac(ctx, db, addr)
return id, errors.WithMessage(err, "no worker found")
}

func init() {
Expand Down
91 changes: 55 additions & 36 deletions grpc-server/template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ import (
)

const (
templateNotFoundID = "abstract-beef-beyond-meat-abominations"
templateNotFoundName = "my-awesome-mock-name"
templateNotFoundTemplate = `version: "0.1"
name: not_found_workflow
global_timeout: 600
tasks:
- name: "not found"
worker: "{{.device_1}}"
actions:
- name: "not_found"
image: not-found
timeout: 60`

templateID1 = "7cd79119-1959-44eb-8b82-bc15bad4888e"
templateName1 = "template_1"
template1 = `version: "0.1"
Expand Down Expand Up @@ -124,6 +137,9 @@ func TestGetTemplate(t *testing.T) {
)
testCases := map[string]struct {
args args
id string
name string
data string
err bool
}{
"SuccessfulTemplateGet_Name": {
Expand All @@ -135,18 +151,18 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Name{Name: templateName1}},
},
err: false,
id: templateID1,
name: templateName1,
data: template1,
err: false,
},

"FailedTemplateGet_Name": {
Expand All @@ -158,18 +174,18 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Name{Name: templateName2}},
},
err: true,
id: templateNotFoundID,
name: templateNotFoundName,
data: templateNotFoundTemplate,
err: true,
},

"SuccessfulTemplateGet_ID": {
Expand All @@ -181,18 +197,18 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Id{Id: templateID1}},
},
err: false,
id: templateID1,
name: templateName1,
data: template1,
err: false,
},

"FailedTemplateGet_ID": {
Expand All @@ -204,18 +220,18 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
getRequest: &pb.GetRequest{GetBy: &pb.GetRequest_Id{Id: templateID2}},
},
err: true,
id: templateNotFoundID,
name: templateNotFoundName,
data: templateNotFoundTemplate,
err: true,
},

"FailedTemplateGet_EmptyRequest": {
Expand All @@ -227,18 +243,18 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
getRequest: &pb.GetRequest{},
},
err: true,
id: templateNotFoundID,
name: templateNotFoundName,
data: templateNotFoundTemplate,
err: true,
},

"FailedTemplateGet_NilRequest": {
Expand All @@ -250,17 +266,17 @@ func TestGetTemplate(t *testing.T) {
GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) {
t.Log("in get template func")

if fields["id"] == templateID1 {
return templateID1, templateName1, template1, nil
}
if fields["name"] == templateName1 {
if fields["id"] == templateID1 || fields["name"] == templateName1 {
return templateID1, templateName1, template1, nil
}
return "", "", "", errors.New("failed to get template")
return templateNotFoundID, templateNotFoundName, templateNotFoundTemplate, errors.New("failed to get template")
},
},
},
err: true,
id: templateNotFoundID,
name: templateNotFoundName,
data: templateNotFoundTemplate,
err: true,
},
}

Expand All @@ -278,6 +294,9 @@ func TestGetTemplate(t *testing.T) {
assert.Nil(t, err)
assert.NotEmpty(t, res)
}
assert.Equal(t, res.Id, tc.id)
assert.Equal(t, res.Name, tc.name)
assert.Equal(t, res.Data, tc.data)
})
}
}
4 changes: 2 additions & 2 deletions grpc-server/workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ var state = map[int32]workflow.State{
}

const (
errFailedToGetTemplate = "failed to get template with ID: %s"
errTemplateParsing = "failed to parse template with ID: %s"
errFailedToGetTemplate = "failed to get template with ID %s"
errTemplateParsing = "failed to parse template with ID %s"
)

// CreateWorkflow implements workflow.CreateWorkflow
Expand Down