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.

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

update status code if action returns error #142

Merged
merged 3 commits into from
Jan 6, 2021

Conversation

ShubhamGupta9582
Copy link
Contributor

@ShubhamGupta9582 ShubhamGupta9582 commented Jan 5, 2021

This PR is a fix for the issue which i raised for the activation's statusCode.
For more details about the issue, please click here

I have updated statusCode on the basis of status So that we can get correct statusCode according to status.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -216,9 +216,28 @@ func (s *ActivationService) Get(activationID string) (*Activation, *http.Respons
return nil, resp, err
}

if a.Success == false {
Copy link
Member

Choose a reason for hiding this comment

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

you could remove this predicate here and instead assign code = 0 for the default case in the switch below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return a, resp, nil
}

func GetStatusCodeForErrorMessage(msg string) (int) {
Copy link
Member

Choose a reason for hiding this comment

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

I suppose it's ok to have this function be public.
You can unit test just this function.
Note that to incorporate this fix into the wsk CLI, we'll need to bump the godep https://github.com/apache/openwhisk-cli/blob/678deee9510abac72b84c7f3ad98a0f58b97443e/Godeps/Godeps.json#L9

@rabbah
Copy link
Member

rabbah commented Jan 6, 2021

@ShubhamGupta9582 this failed in CI:

The command "test -z "$(gofmt -s -l $(echo $GO_FILES))"" failed and exited with 1 during .

I think there's a formatting error. You can run gofmt locally.

@ShubhamGupta9582
Copy link
Contributor Author

ShubhamGupta9582 commented Jan 6, 2021

@rabbah I ran gofmt locally and pushed changes.

EDIT: gofmt command formatted one line but it was not giving any error.
I also ran below two commands on my local and it was working fine

GO_FILES=$(find . -iname '*.go' -type f | grep -v /vendor/)
test -z "$(gofmt -s -l $(echo $GO_FILES))"

How can i produce The command "test -z "$(gofmt -s -l $(echo $GO_FILES))"" failed and exited with 1 during . error on my local ?

@rabbah rabbah merged commit 17d5563 into apache:master Jan 6, 2021
@rabbah
Copy link
Member

rabbah commented Jan 6, 2021

Thanks @ShubhamGupta9582 for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants