You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2020/01/21 03:13:42 UTC
[GitHub] [openwhisk-cli] steven0711dong opened a new pull request #479:
Trigger parameter issue
steven0711dong opened a new pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479
close https://github.com/apache/openwhisk-cli/issues/290, https://github.com/apache/openwhisk-cli/issues/293
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383471254
##########
File path: commands/trigger.go
##########
@@ -547,15 +640,24 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
retTrigger, httpResp, err := Client.Triggers.Get(qualifiedName.GetEntityName())
if err != nil && httpResp.StatusCode == 404 {
- t.Create(Client, args)
+ if createErr := t.Create(Client, args); createErr != nil {
Review comment:
Correct.
1. If trigger get fails with 404, then we create the trigger. If creating the triggers fails, we have to report back to the user. Previously we do not report any error when we create trigger in update and that was a bug.
2. If trigger fails with other reason, execution will fall into next else if, and we report the error message as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383471254
##########
File path: commands/trigger.go
##########
@@ -547,15 +640,24 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
retTrigger, httpResp, err := Client.Triggers.Get(qualifiedName.GetEntityName())
if err != nil && httpResp.StatusCode == 404 {
- t.Create(Client, args)
+ if createErr := t.Create(Client, args); createErr != nil {
Review comment:
Correct.
1. If `trigger get` fails with 404, then we create the trigger. If creating the triggers fails, we have to report back to the user. Previously we do not report any error when we create trigger in update and that was a bug.
2. If `trigger get` fails with other reason, execution will fall into next else if, and we report the error message as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r372038831
##########
File path: commands/trigger.go
##########
@@ -594,10 +579,42 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
}
}
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ errStr := wski18n.T("triggerDoesNotContainFeed", map[string]interface{}{"name": qualifiedName.GetEntityName()})
Review comment:
You are correct. I do not need to add another string in the resource file, so I've changed it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383479392
##########
File path: commands/trigger.go
##########
@@ -397,11 +397,15 @@ func init() {
triggerCreateCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.feed, "feed", "f", "", wski18n.T("trigger feed `ACTION_NAME`"))
+ triggerCreateCmd.Flags().StringSliceVarP(&Flags.trigger.feedParam, "feed-param", "F", []string{}, wski18n.T("feed parameter values in `KEY VALUE` format"))
Review comment:
Are the help entries added to wski18n?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee merged pull request #479: Trigger parameter
issue
Posted by GitBox <gi...@apache.org>.
dubee merged pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383501860
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -491,6 +491,47 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
wsk.trigger.list().stdout should include(triggerName)
}
+ it should "return error message when updating feed param on trigger that contains no feed param" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+ val params = Map("a" -> "A".toJson)
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName, parameters = params)
+ }
+ wsk
+ .cli(
+ Seq("trigger", "update", triggerName, "-F", "feedParam", "feedParamVal", "--auth", wskprops.authKey) ++ wskprops.overrides,
+ expectedExitCode = ERROR_EXIT)
+ .stderr should include("this trigger does not contain a feed")
+ }
+
+ it should "return error message when creating or updating feed with both --param and --trigger-param/--feed-param flags" in withAssetCleaner(
Review comment:
@steven0711dong , I only see `create` tested here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378531063
##########
File path: commands/trigger.go
##########
@@ -436,47 +442,108 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //Within this if statement, we process users' trigger command using the old way.
+ if userIndicatesToUseOldTriggerCommand() {
+ //if user also issued new trigger command then we stop execution
+ if userIssuedNewTriggerCmd() {
+ return nil
+ }
+ // the feed receives all the parameters that are specified on the command line so we merge
+ // the feed lifecycle parameters with the command line ones
+ parameters := getParameters(append(Flags.common.param, additionalFeedParams...), feedQualifiedName == nil, false)
+
+ trigger := &whisk.Trigger{
+ Name: triggerName.GetEntityName(),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ if feedQualifiedName == nil {
+ // parameters are only attached to the trigger in there is no feed, otherwise
+ // parameters are passed to the feed instead
+ trigger.Parameters = parameters.(whisk.KeyValueArr)
+ }
+
+ createOrUpdate(Client, triggerName, trigger, false)
+
+ // Invoke the specified feed action to configure the trigger feed
+ if feedQualifiedName != nil {
+ res, err := invokeAction(*feedQualifiedName, parameters, true, false)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Failed configuring feed '%s' failed: %s\n", feedQualifiedName.GetFullQualifiedName(), err)
+
+ // TODO: should we do this at all? Keeping for now.
+ printFailedBlockingInvocationResponse(*feedQualifiedName, false, res, err)
+
+ reason := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": feedQualifiedName.GetFullQualifiedName(), "err": err})
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": reason})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+
+ // Delete trigger that was created for this feed
+ err = deleteTrigger(triggerName.GetEntityName())
+ if err != nil {
+ whisk.Debug(whisk.DbgWarn, "Ignoring deleteTrigger(%s) failure: %s\n", triggerName.GetEntityName(), err)
+ }
+
+ return werr
+ } else {
+ whisk.Debug(whisk.DbgInfo, "Successfully configured trigger feed via feed action '%s'\n", Flags.common.feed)
+
+ // preserve existing behavior where output of feed activation is emitted to console
+ printInvocationMsg(*feedQualifiedName, true, true, res, color.Output)
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} created trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(trigger.Name)}))
+ return nil
+ }
+
+ //if execution reached this line, it means that users intend to create or update trigger using the new way
+ //by using --feed-param or --trigger-param flag
+ if feedQualifiedName == nil && len(Flags.trigger.feedParam) > 0 {
+ //trigger without feed names cannot have a feed param
+ errMsg := fmt.Sprintf("Incorrect usage. trigger without a feed cannot have feed parameters. \n")
+ fmt.Fprintf(os.Stderr, "%s%s", color.RedString("error: "), errors.New(errMsg))
+ return nil
Review comment:
OK, I'll have both return error.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378519908
##########
File path: commands/commands.go
##########
@@ -181,17 +184,34 @@ func parseArgs(args []string) ([]string, []string, []string, error) {
map[string]interface{}{"err": whiskErr})
whiskErr = whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG,
whisk.DISPLAY_USAGE)
- return nil, nil, nil, whiskErr
+ return nil, nil, nil, nil, nil, whiskErr
}
} else if args[i] == "-a" || args[i] == "--annotation" {
+ fmt.Println("4")
Review comment:
Remove print statement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r371968929
##########
File path: commands/trigger.go
##########
@@ -594,10 +579,42 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
}
}
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ errStr := wski18n.T("triggerDoesNotContainFeed", map[string]interface{}{"name": qualifiedName.GetEntityName()})
Review comment:
`triggerDoesNotContainFeed` represents a very specific message that tells users they've run into a specific situation that will results to error. This happens when they tries to update feed params when the trigger contains no feed. I cannot find any constant in messages.go file that represents this. There is only one constant that's relevant, which is `FEED_CONFIGURATION_FAILURE`, but this is too broad and general error message.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r374892605
##########
File path: commands/commands.go
##########
@@ -181,17 +184,34 @@ func parseArgs(args []string) ([]string, []string, []string, error) {
map[string]interface{}{"err": whiskErr})
whiskErr = whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG,
whisk.DISPLAY_USAGE)
- return nil, nil, nil, whiskErr
+ return nil, nil, nil, nil, nil, whiskErr
}
} else if args[i] == "-a" || args[i] == "--annotation" {
+ fmt.Println("4")
Review comment:
Delete print statement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383459242
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -491,6 +491,47 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
wsk.trigger.list().stdout should include(triggerName)
}
+ it should "return error message when updating feed param on trigger that contains no feed param" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+ val params = Map("a" -> "A".toJson)
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName, parameters = params)
+ }
+ wsk
+ .cli(
+ Seq("trigger", "update", triggerName, "-F", "feedParam", "feedParamVal", "--auth", wskprops.authKey) ++ wskprops.overrides,
+ expectedExitCode = ERROR_EXIT)
+ .stderr should include("this trigger does not contain a feed")
+ }
+
+ it should "return error message when creating or updating feed with both --param and --trigger-param/--feed-param flags" in withAssetCleaner(
Review comment:
Should there also be a test for update?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383542772
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -491,6 +491,47 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
wsk.trigger.list().stdout should include(triggerName)
}
+ it should "return error message when updating feed param on trigger that contains no feed param" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+ val params = Map("a" -> "A".toJson)
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName, parameters = params)
+ }
+ wsk
+ .cli(
+ Seq("trigger", "update", triggerName, "-F", "feedParam", "feedParamVal", "--auth", wskprops.authKey) ++ wskprops.overrides,
+ expectedExitCode = ERROR_EXIT)
+ .stderr should include("this trigger does not contain a feed")
+ }
+
+ it should "return error message when creating or updating feed with both --param and --trigger-param/--feed-param flags" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+
+ var stderr =
Review comment:
I added a test for -P
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r374893115
##########
File path: commands/trigger.go
##########
@@ -397,11 +398,13 @@ func init() {
triggerCreateCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.feed, "feed", "f", "", wski18n.T("trigger feed `ACTION_NAME`"))
+ triggerCreateCmd.Flags().StringSliceVarP(&Flags.trigger.feedParam, "feed-param", "F", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
Review comment:
`feed parameter values` instead of `parameter values` to be explicit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378527279
##########
File path: commands/trigger.go
##########
@@ -436,47 +442,108 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //Within this if statement, we process users' trigger command using the old way.
Review comment:
This is quite a large method with over 100 lines of code. Generally large functions are hard to follow and should be broken out into several functions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] rabbah commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r371932875
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -451,7 +451,7 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
getJSONFromResponse(trigger.stdout, true).fields("parameters") shouldBe JsArray(
JsObject("key" -> JsString("a"), "value" -> JsString("A")))
getJSONFromResponse(trigger.stdout, true).fields("publish") shouldBe false.toJson
- getJSONFromResponse(trigger.stdout, true).fields("version") shouldBe "0.0.2".toJson
+ getJSONFromResponse(trigger.stdout, true).fields("version") shouldBe "0.0.1".toJson
Review comment:
this is semantic changing - it suggests there was a call to trigger update somewhere before that is no longer occurring.
in fact on line 438 and 439 you see there was a `create` and then an `update`. So I think this change is not correct and should be reverted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r375358637
##########
File path: commands/commands.go
##########
@@ -181,17 +184,34 @@ func parseArgs(args []string) ([]string, []string, []string, error) {
map[string]interface{}{"err": whiskErr})
whiskErr = whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG,
whisk.DISPLAY_USAGE)
- return nil, nil, nil, whiskErr
+ return nil, nil, nil, nil, nil, whiskErr
}
} else if args[i] == "-a" || args[i] == "--annotation" {
+ fmt.Println("4")
Review comment:
oops..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r376440012
##########
File path: commands/trigger.go
##########
@@ -436,47 +440,118 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //simplestTrigger indicates user are creating a trigger without any feed or parameters
+ simplestTrigger := len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0
+
+ //if users are 1. creating a trigger without any feed or parameters
+ // 2. creating a trigger using --param flag
+ //then we use the old way to create the trigger.
+ if len(Flags.common.param) > 0 || simplestTrigger {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
Review comment:
Yeah, if users want to create a trigger using the new way. They will create trigger parameters using --trigger-param flag and create feed parameters using --feed-param flag. They just cannot combine --param with --feed-param or --trigger-param. The meaning is ambiguous. So we issue an error here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383502933
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -491,6 +491,47 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
wsk.trigger.list().stdout should include(triggerName)
}
+ it should "return error message when updating feed param on trigger that contains no feed param" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+ val params = Map("a" -> "A".toJson)
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName, parameters = params)
+ }
+ wsk
+ .cli(
+ Seq("trigger", "update", triggerName, "-F", "feedParam", "feedParamVal", "--auth", wskprops.authKey) ++ wskprops.overrides,
+ expectedExitCode = ERROR_EXIT)
+ .stderr should include("this trigger does not contain a feed")
+ }
+
+ it should "return error message when creating or updating feed with both --param and --trigger-param/--feed-param flags" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+
+ var stderr =
Review comment:
Test `-P` as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383543068
##########
File path: commands/trigger.go
##########
@@ -601,3 +703,110 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
return nil
}
+
+//UpdateExtendedVersion only executes when users indicate to update triggers with --feed-param
+//or --trigger-param flags.
+func UpdateExtendedVersion(Client *whisk.Client, args []string, retTrigger *whisk.Trigger) error {
+ var fullFeedName string
+ var qualifiedName = new(QualifiedName)
+ var err error
+
+ if qualifiedName, err = NewQualifiedName(args[0]); err != nil {
+ return NewQualifiedNameError(args[0], err)
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing parameters: %#v\n", Flags.trigger.triggerParam)
+ triggerParameters, err := getJSONFromStrings(Flags.trigger.triggerParam, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.param, err)
+ errStr := wski18n.T("Invalid parameter argument '{{.param}}': {{.err}}",
+ map[string]interface{}{"param": fmt.Sprintf("%#v", Flags.common.param), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing annotations: %#v\n", Flags.common.annotation)
+ annotations, err := getJSONFromStrings(Flags.common.annotation, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.annotation, err)
+ errStr := wski18n.T("Invalid annotation argument '{{.annotation}}': {{.err}}",
+ map[string]interface{}{"annotation": fmt.Sprintf("%#v", Flags.common.annotation), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ trigger := &whisk.Trigger{
+ Name: qualifiedName.GetEntityName(),
+ Parameters: triggerParameters.(whisk.KeyValueArr),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ // Get full feed name from trigger get request as it is needed to get the feed
+ if retTrigger != nil && retTrigger.Annotations != nil {
+ fullFeedName = getValueString(retTrigger.Annotations, "feed")
+ }
+
+ _, _, err = Client.Triggers.Insert(trigger, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v,true) failed: %s\n", trigger, err)
+ errStr := wski18n.T("Unable to update trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ err := errors.New("this trigger does not contain a feed")
+ errStr := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ if len(fullFeedName) > 0 && feedParameterChanged(Flags.trigger.feedParam) {
+ //if there is feed, we invoke the action to configure the feed regardless any changes on feed parameters
+ fullTriggerName := fmt.Sprintf("/%s/%s", qualifiedName.GetNamespace(), qualifiedName.GetEntityName())
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_LIFECYCLE_EVENT, FEED_UPDATE))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_TRIGGER_NAME, fullTriggerName))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_AUTH_KEY, Client.Config.AuthToken))
+
+ // Invoke the specified feed action to configure the trigger feed
+ err = configureFeed(qualifiedName.GetEntityName(), fullFeedName, getParameters(Flags.trigger.feedParam, false, false))
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "configureFeed(%s, %s) failed: %s\n", qualifiedName.GetEntityName(), Flags.common.feed, err)
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} updated trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(qualifiedName.GetEntityName())}))
+
+ return nil
+}
+
+func feedParameterChanged(triggerParam []string) bool {
+ return len(triggerParam) > 0
+}
+
+//if users are 1. creating a trigger without any feed or parameters
+// 2. creating a trigger using --param flag
+//then we use the old way to create the trigger.
+func userIndicatesToUseOldTriggerCommand() bool {
+ return len(Flags.common.param) > 0 || (len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0)
+}
+
+func userIssuedNewTriggerCmd() error {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
+ errStr := wski18n.T("Incorrect usage. Cannot combine --feed-param or --trigger-param flag with --param flag")
Review comment:
made error msg more generic
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383476638
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -491,6 +491,47 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
wsk.trigger.list().stdout should include(triggerName)
}
+ it should "return error message when updating feed param on trigger that contains no feed param" in withAssetCleaner(
+ wskprops) { (wp, assetHelper) =>
+ val triggerName = withTimestamp("t1tor1")
+ val ns = wsk.namespace.whois()
+ val params = Map("a" -> "A".toJson)
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName, parameters = params)
+ }
+ wsk
+ .cli(
+ Seq("trigger", "update", triggerName, "-F", "feedParam", "feedParamVal", "--auth", wskprops.authKey) ++ wskprops.overrides,
+ expectedExitCode = ERROR_EXIT)
+ .stderr should include("this trigger does not contain a feed")
+ }
+
+ it should "return error message when creating or updating feed with both --param and --trigger-param/--feed-param flags" in withAssetCleaner(
Review comment:
This test case is for both create and update.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383542835
##########
File path: commands/trigger.go
##########
@@ -397,11 +397,15 @@ func init() {
triggerCreateCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.feed, "feed", "f", "", wski18n.T("trigger feed `ACTION_NAME`"))
+ triggerCreateCmd.Flags().StringSliceVarP(&Flags.trigger.feedParam, "feed-param", "F", []string{}, wski18n.T("feed parameter values in `KEY VALUE` format"))
Review comment:
added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] rabbah commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r371933869
##########
File path: commands/trigger.go
##########
@@ -594,10 +579,42 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
}
}
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ errStr := wski18n.T("triggerDoesNotContainFeed", map[string]interface{}{"name": qualifiedName.GetEntityName()})
Review comment:
Please look at using a constant in https://github.com/apache/openwhisk-cli/blob/master/commands/messages.go instead of a string `triggerDoesNotContainFeed` which requires a matching entry in the localization file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383479086
##########
File path: commands/trigger.go
##########
@@ -601,3 +703,110 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
return nil
}
+
+//UpdateExtendedVersion only executes when users indicate to update triggers with --feed-param
+//or --trigger-param flags.
+func UpdateExtendedVersion(Client *whisk.Client, args []string, retTrigger *whisk.Trigger) error {
+ var fullFeedName string
+ var qualifiedName = new(QualifiedName)
+ var err error
+
+ if qualifiedName, err = NewQualifiedName(args[0]); err != nil {
+ return NewQualifiedNameError(args[0], err)
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing parameters: %#v\n", Flags.trigger.triggerParam)
+ triggerParameters, err := getJSONFromStrings(Flags.trigger.triggerParam, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.param, err)
+ errStr := wski18n.T("Invalid parameter argument '{{.param}}': {{.err}}",
+ map[string]interface{}{"param": fmt.Sprintf("%#v", Flags.common.param), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing annotations: %#v\n", Flags.common.annotation)
+ annotations, err := getJSONFromStrings(Flags.common.annotation, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.annotation, err)
+ errStr := wski18n.T("Invalid annotation argument '{{.annotation}}': {{.err}}",
+ map[string]interface{}{"annotation": fmt.Sprintf("%#v", Flags.common.annotation), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ trigger := &whisk.Trigger{
+ Name: qualifiedName.GetEntityName(),
+ Parameters: triggerParameters.(whisk.KeyValueArr),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ // Get full feed name from trigger get request as it is needed to get the feed
+ if retTrigger != nil && retTrigger.Annotations != nil {
+ fullFeedName = getValueString(retTrigger.Annotations, "feed")
+ }
+
+ _, _, err = Client.Triggers.Insert(trigger, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v,true) failed: %s\n", trigger, err)
+ errStr := wski18n.T("Unable to update trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ err := errors.New("this trigger does not contain a feed")
+ errStr := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ if len(fullFeedName) > 0 && feedParameterChanged(Flags.trigger.feedParam) {
+ //if there is feed, we invoke the action to configure the feed regardless any changes on feed parameters
+ fullTriggerName := fmt.Sprintf("/%s/%s", qualifiedName.GetNamespace(), qualifiedName.GetEntityName())
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_LIFECYCLE_EVENT, FEED_UPDATE))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_TRIGGER_NAME, fullTriggerName))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_AUTH_KEY, Client.Config.AuthToken))
+
+ // Invoke the specified feed action to configure the trigger feed
+ err = configureFeed(qualifiedName.GetEntityName(), fullFeedName, getParameters(Flags.trigger.feedParam, false, false))
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "configureFeed(%s, %s) failed: %s\n", qualifiedName.GetEntityName(), Flags.common.feed, err)
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} updated trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(qualifiedName.GetEntityName())}))
+
+ return nil
+}
+
+func feedParameterChanged(triggerParam []string) bool {
+ return len(triggerParam) > 0
+}
+
+//if users are 1. creating a trigger without any feed or parameters
+// 2. creating a trigger using --param flag
+//then we use the old way to create the trigger.
+func userIndicatesToUseOldTriggerCommand() bool {
+ return len(Flags.common.param) > 0 || (len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0)
+}
+
+func userIssuedNewTriggerCmd() error {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
+ errStr := wski18n.T("Incorrect usage. Cannot combine --feed-param or --trigger-param flag with --param flag")
Review comment:
Need to say `--param-file` to this list. Is this error message exported to wski18n?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r375341360
##########
File path: commands/trigger.go
##########
@@ -436,47 +440,118 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //simplestTrigger indicates user are creating a trigger without any feed or parameters
+ simplestTrigger := len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0
+
+ //if users are 1. creating a trigger without any feed or parameters
+ // 2. creating a trigger using --param flag
+ //then we use the old way to create the trigger.
+ if len(Flags.common.param) > 0 || simplestTrigger {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
Review comment:
Is there a use case for the user trying to use trigger parameters and feed parameters together?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378520086
##########
File path: commands/commands.go
##########
@@ -19,6 +19,7 @@ package commands
import (
"errors"
+ "fmt"
Review comment:
Remove `fmt`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378524547
##########
File path: commands/trigger.go
##########
@@ -597,7 +679,75 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
fmt.Fprintf(color.Output,
wski18n.T("{{.ok}} updated trigger {{.name}}\n",
map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(qualifiedName.GetEntityName())}))
+
+ return nil
}
+ //if execution reached this line, it means that users intend to create or update trigger using the new way
+ //by using --feed-param or --trigger-param flag
+ trigger.Parameters = triggerParameters.(whisk.KeyValueArr)
+ _, _, err = Client.Triggers.Insert(trigger, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v,true) failed: %s\n", trigger, err)
+ errStr := wski18n.T("Unable to update trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ err := errors.New("this trigger does not contain a feed")
+ errStr := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ if len(fullFeedName) > 0 && feedParameterChanged(Flags.trigger.feedParam) {
+ //if there is feed, we invoke the action to configure the feed regardless any changes on feed parameters
+ fullTriggerName := fmt.Sprintf("/%s/%s", qualifiedName.GetNamespace(), qualifiedName.GetEntityName())
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_LIFECYCLE_EVENT, FEED_UPDATE))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_TRIGGER_NAME, fullTriggerName))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_AUTH_KEY, Client.Config.AuthToken))
+
+ // Invoke the specified feed action to configure the trigger feed
+ err = configureFeed(qualifiedName.GetEntityName(), fullFeedName, getParameters(Flags.trigger.feedParam, false, false))
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "configureFeed(%s, %s) failed: %s\n", qualifiedName.GetEntityName(), Flags.common.feed, err)
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} updated trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(qualifiedName.GetEntityName())}))
+
return nil
}
+
+func feedParameterChanged(triggerParam []string) bool {
+ return len(triggerParam) > 0
+}
+
+//if users are 1. creating a trigger without any feed or parameters
+// 2. creating a trigger using --param flag
+//then we use the old way to create the trigger.
+func userIndicatesToUseOldTriggerCommand() bool {
+ return len(Flags.common.param) > 0 || (len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0)
+}
+
+func userIssuedNewTriggerCmd() bool {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
+ errMsg := fmt.Sprintf("Incorrect usage. Cannot combine --feed-param or --trigger-param flag with --param flag\n")
+ fmt.Fprintf(os.Stderr, "%s%s", color.RedString("error: "), errors.New(errMsg))
Review comment:
Errors are usually returned to the system, which halts further execution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong edited a comment on issue #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong edited a comment on issue #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#issuecomment-585462174
I've already changed the error message thing, I'll look at how to break them into small functions tomorrow
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383552879
##########
File path: wski18n/resources/en_US.all.json
##########
@@ -1594,5 +1594,21 @@
{
"id": "CMD_DESC_LONG_EXPORT",
"translation": "Exports managed project assets from OpenWhisk to manifest and function files.\n\nThe most common way to run export:\n$ wsk project export --projectname PROJECT --manifest path/to/exported-manifest.yaml"
+ },
+ {
+ "id": "Incorrect usage. Cannot combine --feed-param or --trigger-param flag with neither --param nor --param-file flag",
+ "translation": "Incorrect usage. Cannot combine --feed-param or --trigger-param flag with neither --param nor --param-file flag"
+ },
+ {
+ "id": "trigger parameter values in `KEY VALUE` format",
+ "translation": "trigger parameter values in `KEY VALUE` format"
+ },
+ {
+ "id": "feed parameter values in `KEY VALUE` format",
+ "translation": "feed parameter values in `KEY VALUE` format"
+ },
+ {
+ "id": "Incorrect usage. trigger without a feed cannot have feed parameters",
Review comment:
Capital `t` in `trigger.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r376517816
##########
File path: commands/trigger.go
##########
@@ -436,47 +440,118 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //simplestTrigger indicates user are creating a trigger without any feed or parameters
Review comment:
I created two functions so that both create and update could use. Some other variables such as params, feed params do exist in both functions but are created differently and the creation depends on users' intention on how to issue trigger command(old way and new way). So it might make things more complicated to create functions out of duplicate code.
I have to admit that after adding the new flags, both functions become bigger but I think it might be a good idea to separate execution path for the old way of creating trigger from the new way so they do not get lumped together. I'll update the PR this afternoon.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r376517816
##########
File path: commands/trigger.go
##########
@@ -436,47 +440,118 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //simplestTrigger indicates user are creating a trigger without any feed or parameters
Review comment:
I created two functions so that both create and update could use. Some other variables such as params, feed params do exist in both functions but are created differently and the creation depends on users' intention on how to issue trigger command(old way and new way). So it might make things more complicated to create functions out of duplicate code.
I have to admit that after adding the new flags, both functions become bigger but I think it might be a good idea to separate execution path for the old way of creating trigger from the new way so they do not get lumped together.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383471254
##########
File path: commands/trigger.go
##########
@@ -547,15 +640,24 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
retTrigger, httpResp, err := Client.Triggers.Get(qualifiedName.GetEntityName())
if err != nil && httpResp.StatusCode == 404 {
- t.Create(Client, args)
+ if createErr := t.Create(Client, args); createErr != nil {
Review comment:
Correct.
1. If `trigger get` fails with 404, then we create the trigger. If creating the triggers fails, we have to report back to the user. Previously we do not report any error when we create trigger in update and that was a bug.
2. If trigger fails with other reason, execution will fall into next else if, and we report the error message as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r375344993
##########
File path: commands/trigger.go
##########
@@ -436,47 +440,118 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //simplestTrigger indicates user are creating a trigger without any feed or parameters
Review comment:
Does duplicate code exist in create and update? If so it'd be better to create functions out of the duplicate code to reduce complexity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r372090479
##########
File path: tests/src/test/scala/system/basic/WskCliBasicTests.scala
##########
@@ -451,7 +451,7 @@ class WskCliBasicTests extends TestHelpers with WskTestHelpers {
getJSONFromResponse(trigger.stdout, true).fields("parameters") shouldBe JsArray(
JsObject("key" -> JsString("a"), "value" -> JsString("A")))
getJSONFromResponse(trigger.stdout, true).fields("publish") shouldBe false.toJson
- getJSONFromResponse(trigger.stdout, true).fields("version") shouldBe "0.0.2".toJson
+ getJSONFromResponse(trigger.stdout, true).fields("version") shouldBe "0.0.1".toJson
Review comment:
Nice catch!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378528233
##########
File path: commands/trigger.go
##########
@@ -548,17 +623,30 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
if err != nil && httpResp.StatusCode == 404 {
t.Create(Client, args)
+ return nil
} else if err != nil {
whisk.Debug(whisk.DbgError, "Client.Triggers.Get(%s) failed: %s\n", qualifiedName.GetEntityName(), err)
errStr := wski18n.T("Unable to get trigger '{{.name}}': {{.err}}",
map[string]interface{}{"name": qualifiedName.GetEntityName(), "err": err})
werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
return werr
- } else {
+ }
- // Get full feed name from trigger get request as it is needed to get the feed
- if retTrigger != nil && retTrigger.Annotations != nil {
- fullFeedName = getValueString(retTrigger.Annotations, "feed")
+ // Get full feed name from trigger get request as it is needed to get the feed
Review comment:
This this update method has 200 lines of code making it extremely hard to understand quickly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r372038437
##########
File path: commands/commands.go
##########
@@ -191,7 +192,17 @@ func parseArgs(args []string) ([]string, []string, []string, error) {
map[string]interface{}{"err": whiskErr})
whiskErr = whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG,
whisk.DISPLAY_USAGE)
- return nil, nil, nil, whiskErr
+ return nil, nil, nil, nil, whiskErr
+ }
+ } else if args[i] == "-fp" || args[i] == "--feed-param" {
+ feedParamArgs, args, whiskErr = getKeyValueArgs(args, i, feedParamArgs)
+ if whiskErr != nil {
+ whisk.Debug(whisk.DbgError, "getKeyValueArgs(%#v, %d) failed: %s\n", args, i, whiskErr)
+ errMsg := wski18n.T("The parameter arguments are invalid: {{.err}}",
Review comment:
I realize that it is sufficient to rely on error message produced by getKeyValueArgs. So I removed the unnecessary error string.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383983553
##########
File path: commands/trigger.go
##########
@@ -431,52 +435,141 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return whiskErr
}
+ //1. if the command line arguments user provides contains only --param flags
+ //2. if the command line arguments user provides contains no --param flags at all
+ //we should process the trigger create command in the old way.
+ if userIndicatesToUseOldTriggerCommand() {
+ triggerName, err := NewQualifiedName(args[0])
+ if err != nil {
+ return NewQualifiedNameError(args[0], err)
+ }
+
+ //if user also issued new trigger command then we stop execution
+ if triggerUsageErr := userIssuedNewTriggerCmd(); triggerUsageErr != nil {
+ return triggerUsageErr
+ }
+
+ annotationArray := Flags.common.annotation
+ authToken := Client.Config.AuthToken
+
+ // if a feed is specified, create additional parameters which must be passed to the feed
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
+
+ // if a feed is specified, add feed annotation the annotations declared on the command line
+ // TODO: add test to ensure that generated annotation has precedence
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
+ }
+ annotations := getParameters(annotationArray, true, true)
+
+ // the feed receives all the parameters that are specified on the command line so we merge
+ // the feed lifecycle parameters with the command line ones
+ parameters := getParameters(append(Flags.common.param, additionalFeedParams...), feedQualifiedName == nil, false)
+
+ trigger := &whisk.Trigger{
+ Name: triggerName.GetEntityName(),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ if feedQualifiedName == nil {
+ // parameters are only attached to the trigger in there is no feed, otherwise
+ // parameters are passed to the feed instead
+ trigger.Parameters = parameters.(whisk.KeyValueArr)
+ }
+
+ createOrUpdate(Client, triggerName, trigger, false)
+
+ // Invoke the specified feed action to configure the trigger feed
+ if feedQualifiedName != nil {
+ res, err := invokeAction(*feedQualifiedName, parameters, true, false)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Failed configuring feed '%s' failed: %s\n", feedQualifiedName.GetFullQualifiedName(), err)
+
+ // TODO: should we do this at all? Keeping for now.
+ printFailedBlockingInvocationResponse(*feedQualifiedName, false, res, err)
+
+ reason := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": feedQualifiedName.GetFullQualifiedName(), "err": err})
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": reason})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+
+ // Delete trigger that was created for this feed
+ err = deleteTrigger(triggerName.GetEntityName())
+ if err != nil {
+ whisk.Debug(whisk.DbgWarn, "Ignoring deleteTrigger(%s) failure: %s\n", triggerName.GetEntityName(), err)
+ }
+
+ return werr
+ } else {
+ whisk.Debug(whisk.DbgInfo, "Successfully configured trigger feed via feed action '%s'\n", Flags.common.feed)
+
+ // preserve existing behavior where output of feed activation is emitted to console
+ printInvocationMsg(*feedQualifiedName, true, true, res, color.Output)
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} created trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(trigger.Name)}))
+ return nil
+ }
+ //1. if user's input command line argument contains either --feed-param or --trigger-param
+ //2. if user's input command line argument contains both --feed-param and --trigger-param
+ //then we process trigger create command in a different way
+ return CreateExtendedVersion(Client, args)
+}
+
+//CreateExtendedVersion only executes when users indicate to create triggers with --feed-param
+//or --trigger-param flags.
+func CreateExtendedVersion(Client *whisk.Client, args []string) error {
triggerName, err := NewQualifiedName(args[0])
if err != nil {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
+
annotations := getParameters(annotationArray, true, true)
+ //if trigger contains no feed but user tries to update feed parameter, then we issue error.
+ if feedQualifiedName == nil && len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "Incorrect usage. trigger without a feed cannot have feed parameters")
Review comment:
This error string does not match what is in `en_US.all.json`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r374893356
##########
File path: commands/trigger.go
##########
@@ -397,11 +398,13 @@ func init() {
triggerCreateCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
triggerCreateCmd.Flags().StringVarP(&Flags.common.feed, "feed", "f", "", wski18n.T("trigger feed `ACTION_NAME`"))
+ triggerCreateCmd.Flags().StringSliceVarP(&Flags.trigger.feedParam, "feed-param", "F", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerUpdateCmd.Flags().StringSliceVarP(&Flags.common.annotation, "annotation", "a", []string{}, wski18n.T("annotation values in `KEY VALUE` format"))
triggerUpdateCmd.Flags().StringVarP(&Flags.common.annotFile, "annotation-file", "A", "", wski18n.T("`FILE` containing annotation values in JSON format"))
triggerUpdateCmd.Flags().StringSliceVarP(&Flags.common.param, "param", "p", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
triggerUpdateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
+ triggerUpdateCmd.Flags().StringSliceVarP(&Flags.trigger.feedParam, "feed-param", "F", []string{}, wski18n.T("parameter values in `KEY VALUE` format"))
Review comment:
`feed parameter values` instead of `parameter values` to be explicit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378523913
##########
File path: commands/trigger.go
##########
@@ -436,47 +442,108 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //Within this if statement, we process users' trigger command using the old way.
+ if userIndicatesToUseOldTriggerCommand() {
+ //if user also issued new trigger command then we stop execution
+ if userIssuedNewTriggerCmd() {
+ return nil
+ }
+ // the feed receives all the parameters that are specified on the command line so we merge
+ // the feed lifecycle parameters with the command line ones
+ parameters := getParameters(append(Flags.common.param, additionalFeedParams...), feedQualifiedName == nil, false)
+
+ trigger := &whisk.Trigger{
+ Name: triggerName.GetEntityName(),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ if feedQualifiedName == nil {
+ // parameters are only attached to the trigger in there is no feed, otherwise
+ // parameters are passed to the feed instead
+ trigger.Parameters = parameters.(whisk.KeyValueArr)
+ }
+
+ createOrUpdate(Client, triggerName, trigger, false)
+
+ // Invoke the specified feed action to configure the trigger feed
+ if feedQualifiedName != nil {
+ res, err := invokeAction(*feedQualifiedName, parameters, true, false)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Failed configuring feed '%s' failed: %s\n", feedQualifiedName.GetFullQualifiedName(), err)
+
+ // TODO: should we do this at all? Keeping for now.
+ printFailedBlockingInvocationResponse(*feedQualifiedName, false, res, err)
+
+ reason := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": feedQualifiedName.GetFullQualifiedName(), "err": err})
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": reason})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+
+ // Delete trigger that was created for this feed
+ err = deleteTrigger(triggerName.GetEntityName())
+ if err != nil {
+ whisk.Debug(whisk.DbgWarn, "Ignoring deleteTrigger(%s) failure: %s\n", triggerName.GetEntityName(), err)
+ }
+
+ return werr
+ } else {
+ whisk.Debug(whisk.DbgInfo, "Successfully configured trigger feed via feed action '%s'\n", Flags.common.feed)
+
+ // preserve existing behavior where output of feed activation is emitted to console
+ printInvocationMsg(*feedQualifiedName, true, true, res, color.Output)
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} created trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(trigger.Name)}))
+ return nil
+ }
+
+ //if execution reached this line, it means that users intend to create or update trigger using the new way
+ //by using --feed-param or --trigger-param flag
+ if feedQualifiedName == nil && len(Flags.trigger.feedParam) > 0 {
+ //trigger without feed names cannot have a feed param
+ errMsg := fmt.Sprintf("Incorrect usage. trigger without a feed cannot have feed parameters. \n")
+ fmt.Fprintf(os.Stderr, "%s%s", color.RedString("error: "), errors.New(errMsg))
+ return nil
Review comment:
Hmm, errors are usually return back to the system instead of `nil`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383552879
##########
File path: wski18n/resources/en_US.all.json
##########
@@ -1594,5 +1594,21 @@
{
"id": "CMD_DESC_LONG_EXPORT",
"translation": "Exports managed project assets from OpenWhisk to manifest and function files.\n\nThe most common way to run export:\n$ wsk project export --projectname PROJECT --manifest path/to/exported-manifest.yaml"
+ },
+ {
+ "id": "Incorrect usage. Cannot combine --feed-param or --trigger-param flag with neither --param nor --param-file flag",
+ "translation": "Incorrect usage. Cannot combine --feed-param or --trigger-param flag with neither --param nor --param-file flag"
+ },
+ {
+ "id": "trigger parameter values in `KEY VALUE` format",
+ "translation": "trigger parameter values in `KEY VALUE` format"
+ },
+ {
+ "id": "feed parameter values in `KEY VALUE` format",
+ "translation": "feed parameter values in `KEY VALUE` format"
+ },
+ {
+ "id": "Incorrect usage. trigger without a feed cannot have feed parameters",
Review comment:
Capital `t` in `trigger`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] rabbah commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r371931439
##########
File path: commands/commands.go
##########
@@ -191,7 +192,17 @@ func parseArgs(args []string) ([]string, []string, []string, error) {
map[string]interface{}{"err": whiskErr})
whiskErr = whisk.MakeWskError(errors.New(errMsg), whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG,
whisk.DISPLAY_USAGE)
- return nil, nil, nil, whiskErr
+ return nil, nil, nil, nil, whiskErr
+ }
+ } else if args[i] == "-fp" || args[i] == "--feed-param" {
+ feedParamArgs, args, whiskErr = getKeyValueArgs(args, i, feedParamArgs)
+ if whiskErr != nil {
+ whisk.Debug(whisk.DbgError, "getKeyValueArgs(%#v, %d) failed: %s\n", args, i, whiskErr)
+ errMsg := wski18n.T("The parameter arguments are invalid: {{.err}}",
Review comment:
I know this is mostly copy/paste but if you can instead use a constant in https://github.com/apache/openwhisk-cli/blob/master/commands/messages.go we can avoid replicating this error message in several places.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383479086
##########
File path: commands/trigger.go
##########
@@ -601,3 +703,110 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
return nil
}
+
+//UpdateExtendedVersion only executes when users indicate to update triggers with --feed-param
+//or --trigger-param flags.
+func UpdateExtendedVersion(Client *whisk.Client, args []string, retTrigger *whisk.Trigger) error {
+ var fullFeedName string
+ var qualifiedName = new(QualifiedName)
+ var err error
+
+ if qualifiedName, err = NewQualifiedName(args[0]); err != nil {
+ return NewQualifiedNameError(args[0], err)
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing parameters: %#v\n", Flags.trigger.triggerParam)
+ triggerParameters, err := getJSONFromStrings(Flags.trigger.triggerParam, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.param, err)
+ errStr := wski18n.T("Invalid parameter argument '{{.param}}': {{.err}}",
+ map[string]interface{}{"param": fmt.Sprintf("%#v", Flags.common.param), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ whisk.Debug(whisk.DbgInfo, "Parsing annotations: %#v\n", Flags.common.annotation)
+ annotations, err := getJSONFromStrings(Flags.common.annotation, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "getJSONFromStrings(%#v, true) failed: %s\n", Flags.common.annotation, err)
+ errStr := wski18n.T("Invalid annotation argument '{{.annotation}}': {{.err}}",
+ map[string]interface{}{"annotation": fmt.Sprintf("%#v", Flags.common.annotation), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
+ return werr
+ }
+
+ trigger := &whisk.Trigger{
+ Name: qualifiedName.GetEntityName(),
+ Parameters: triggerParameters.(whisk.KeyValueArr),
+ Annotations: annotations.(whisk.KeyValueArr),
+ }
+
+ // Get full feed name from trigger get request as it is needed to get the feed
+ if retTrigger != nil && retTrigger.Annotations != nil {
+ fullFeedName = getValueString(retTrigger.Annotations, "feed")
+ }
+
+ _, _, err = Client.Triggers.Insert(trigger, true)
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "Client.Triggers.Insert(%+v,true) failed: %s\n", trigger, err)
+ errStr := wski18n.T("Unable to update trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": trigger.Name, "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ //if there is no feed attached to this trigger
+ if len(fullFeedName) < 1 {
+ //but user indicate feed parameter change, we issue error message.
+ if len(Flags.trigger.feedParam) > 0 {
+ whisk.Debug(whisk.DbgError, "trigger %+v is found but it does not contain a feed. \n", trigger)
+ err := errors.New("this trigger does not contain a feed")
+ errStr := wski18n.T(FEED_CONFIGURATION_FAILURE, map[string]interface{}{"feedname": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ if len(fullFeedName) > 0 && feedParameterChanged(Flags.trigger.feedParam) {
+ //if there is feed, we invoke the action to configure the feed regardless any changes on feed parameters
+ fullTriggerName := fmt.Sprintf("/%s/%s", qualifiedName.GetNamespace(), qualifiedName.GetEntityName())
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_LIFECYCLE_EVENT, FEED_UPDATE))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_TRIGGER_NAME, fullTriggerName))
+ Flags.trigger.feedParam = append(Flags.trigger.feedParam, getFormattedJSON(FEED_AUTH_KEY, Client.Config.AuthToken))
+
+ // Invoke the specified feed action to configure the trigger feed
+ err = configureFeed(qualifiedName.GetEntityName(), fullFeedName, getParameters(Flags.trigger.feedParam, false, false))
+ if err != nil {
+ whisk.Debug(whisk.DbgError, "configureFeed(%s, %s) failed: %s\n", qualifiedName.GetEntityName(), Flags.common.feed, err)
+ errStr := wski18n.T("Unable to create trigger '{{.name}}': {{.err}}",
+ map[string]interface{}{"name": qualifiedName.GetEntityName(), "err": err})
+ werr := whisk.MakeWskErrorFromWskError(errors.New(errStr), err, whisk.EXIT_CODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.NO_DISPLAY_USAGE)
+ return werr
+ }
+ }
+
+ fmt.Fprintf(color.Output,
+ wski18n.T("{{.ok}} updated trigger {{.name}}\n",
+ map[string]interface{}{"ok": color.GreenString("ok:"), "name": boldString(qualifiedName.GetEntityName())}))
+
+ return nil
+}
+
+func feedParameterChanged(triggerParam []string) bool {
+ return len(triggerParam) > 0
+}
+
+//if users are 1. creating a trigger without any feed or parameters
+// 2. creating a trigger using --param flag
+//then we use the old way to create the trigger.
+func userIndicatesToUseOldTriggerCommand() bool {
+ return len(Flags.common.param) > 0 || (len(Flags.trigger.feedParam) == 0 && len(Flags.common.param) == 0 && len(Flags.trigger.triggerParam) == 0)
+}
+
+func userIssuedNewTriggerCmd() error {
+ if len(Flags.trigger.feedParam) > 0 || len(Flags.trigger.triggerParam) > 0 {
+ whisk.Debug(whisk.DbgError, "User tries to mix use of --param with --feed-param and --trigger-param")
+ errStr := wski18n.T("Incorrect usage. Cannot combine --feed-param or --trigger-param flag with --param flag")
Review comment:
Need to add `--param-file` to this list. Is this error message exported to wski18n?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on issue #479: Trigger
parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on issue #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#issuecomment-585462174
I've already changed the error message thing, I'll look at how to break them into small functions tonight.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] dubee commented on a change in pull request #479:
Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
dubee commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r383463938
##########
File path: commands/trigger.go
##########
@@ -547,15 +640,24 @@ func (t *Trigger) Update(Client *whisk.Client, args []string) error {
retTrigger, httpResp, err := Client.Triggers.Get(qualifiedName.GetEntityName())
if err != nil && httpResp.StatusCode == 404 {
- t.Create(Client, args)
+ if createErr := t.Create(Client, args); createErr != nil {
Review comment:
This is new. If trigger get fails then trigger update now fails, correct?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-cli] steven0711dong commented on a change in pull
request #479: Trigger parameter issue
Posted by GitBox <gi...@apache.org>.
steven0711dong commented on a change in pull request #479: Trigger parameter issue
URL: https://github.com/apache/openwhisk-cli/pull/479#discussion_r378533246
##########
File path: commands/trigger.go
##########
@@ -436,47 +442,108 @@ func (t *Trigger) Create(Client *whisk.Client, args []string) error {
return NewQualifiedNameError(args[0], err)
}
- paramArray := Flags.common.param
annotationArray := Flags.common.annotation
- feedParam := Flags.common.feed
authToken := Client.Config.AuthToken
// if a feed is specified, create additional parameters which must be passed to the feed
- feedName, feedParams := feedParameters(feedParam, FEED_CREATE, triggerName, authToken)
- // the feed receives all the parameters that are specified on the command line so we merge
- // the feed lifecycle parameters with the command line ones
- parameters := getParameters(append(paramArray, feedParams...), feedName == nil, false)
+ feedQualifiedName, additionalFeedParams := feedParameters(Flags.common.feed, FEED_CREATE, triggerName, authToken)
// if a feed is specified, add feed annotation the annotations declared on the command line
// TODO: add test to ensure that generated annotation has precedence
- if feedName != nil {
- annotationArray = append(annotationArray, getFormattedJSON("feed", feedName.GetFullQualifiedName()))
+ if feedQualifiedName != nil {
+ annotationArray = append(annotationArray, getFormattedJSON("feed", feedQualifiedName.GetFullQualifiedName()))
}
annotations := getParameters(annotationArray, true, true)
+ //Within this if statement, we process users' trigger command using the old way.
Review comment:
I can try to separate the 'old way' and 'new way' into two different functions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services