Hi Steven. Thanks for outlining the change. I have looked at the PR and I believe one of the changes in the test is not sound. Please take a look. In general I think this is a good direction but will require quickly updating the documentation because all the examples for creating a feed *will* break.
-r On Fri, Jan 24, 2020 at 10:49 AM Steven Dong <steven7d...@gmail.com> wrote: > Hello, > > My name is Steven Dong and I am sending this email to describe some changes > I've made for Openwhisk-CLI repository on wsk trigger create and update > issue. The reason why I make this change is because there are outstanding > git issues for this and it looks like the issues have been out there for a > while. Also, I've had people around me frequently using trigger command > making some complaint about this. The outstanding git issues are here: > > https://github.com/apache/openwhisk-cli/issues/290 > https://github.com/apache/openwhisk-cli/issues/293 > > I have already made the changes and created a PR for it. The PR is here: > https://github.com/apache/openwhisk-cli/pull/479. The PR has passed the > Travis build and test and can be merged. The change I made is: adding > --feed-param flag to trigger create and trigger update command so that we > can distinguish parameters for trigger and trigger feed. Currently, all > parameters on trigger feed creation and update are passed to feed action > and our trigger create and update command does not digest user supplied > parameters properly for the above mentioned reason. For example, creating a > trigger with an alarm feed that fires every minute would be the following: > > wsk trigger create testTrigger --param triggerParam "this is from trigger > parameter" --feed "/whisk.system/alarms/alarm" --param trigger_payload > "{filename: sampledata.cvs}" --param cron > > "0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59 > 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23 * * > 0,1,2,3,4,5,6" > ` > This creates the trigger with correctly configured alarm feed but the > created trigger is missing its trigger paramete: triggerParam "this is from > trigger parameter". Also, we have no way of updating the trigger parameter > on a trigger with feed. The upcoming change would allow users to do exact > the same thing using the following command: > > wsk trigger create testTrigger --param triggerParam "this is from trigger > parameter" --feed "/whisk.system/alarms/alarm" --feed-param trigger_payload > "{filename: sampledata.cvs}" --feed-param cron > > "0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59 > 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23 * * > 0,1,2,3,4,5,6" > > If user wants to update trigger parameters, they can do: `wsk trigger > update testTrigger --param newTriggerParam "this is new trigger parameter"` > If user wants to update alarm feed cron parameters, they can do: `wsk > trigger update testTrigger --feed-param cron "0,1,2,3,4......" ` > > Feel free to clone my branch and have a local build to run and test out the > new changes. > > -Regards, > > Steven Dong >
