Skip to content

Conversation

@dubee
Copy link
Member

@dubee dubee commented Jun 4, 2018

When a trigger without active or associated rule(s) is fired, an activation ID is not returned from an OpenWhisk server. These changes display an appropriate message when such an scenario occurs.

Closes #301

@dubee dubee requested a review from mdeuser June 4, 2018 04:49
@dubee dubee force-pushed the trigger-fire-message branch from 598219f to b83acb7 Compare June 4, 2018 04:50
return werr
}

if resp.StatusCode == 204 {
Copy link
Contributor

Choose a reason for hiding this comment

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

if resp.StatusCode == http.StatusNoContent {

"translation": "{{.ok}} triggered /{{.namespace}}/{{.name}} with id {{.id}}\n"
},
{
"id": "trigger /{{.namespace}}/{{.name}} did not fire as it is not associated with an active rule(s)\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

i think rules(s) can be changed to rule. only one active rule is needed.

}

val rr = wsk.trigger.fire(triggerName)
val ns = wsk.namespace.whois()
Copy link
Contributor

Choose a reason for hiding this comment

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

not used?

"namespace": boldString(qualifiedName.GetNamespace()),
"name": boldString(qualifiedName.GetEntityName())}))

return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

up for discussion... is this situation considered an error (i.e. non-zero exit code, stderr)?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mdeuser, I believe we agreed to return a non-zero exit code here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dubee yeah.. that's what you and i agreed upon. was looking if anyone had an opposite argument :-)

Copy link
Member

Choose a reason for hiding this comment

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

which exit code would you return?


if resp.StatusCode == http.StatusNoContent {
fmt.Fprintf(color.Output,
wski18n.T("trigger /{{.namespace}}/{{.name}} did not fire as it is not associated with an active rule\n",
Copy link
Member

Choose a reason for hiding this comment

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

I suggest for all new message and to start transitioning these message ids to constants - see for example baa07f0.

return werr
}

if resp.StatusCode == http.StatusNoContent {
Copy link
Member

Choose a reason for hiding this comment

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

I suggest starting to add go unit tests as well for such changes as it will vector toward better factoring of the code. For example a unit test that mocks an HTTP response and confirms returning the expected message and CLI code.

@dubee dubee force-pushed the trigger-fire-message branch from bb32c44 to 3031300 Compare August 10, 2018 02:56
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.

3 participants