apache / openwhisk

Apache OpenWhisk is an open source serverless cloud platform
https://openwhisk.apache.org/
Apache License 2.0
6.5k stars 1.16k forks source link

confusing error message for wsk action update #38

Closed sjfink closed 7 years ago

sjfink commented 8 years ago

Attempting to update an action sequence that doesn't exist results in a confusing error message:

wsk: error: unrecognized arguments:

We need a better error message like "Error: cannot update sequence A which does not exist".

Alternatively, we could change the semantics of 'update' to mean 'createOrUpdate'

MacBook-Pro-3:~ sjfink$ wsk list
entities in namespace: sjfink@us.ibm.com_dev
packages
/sjfink@us.ibm.com_dev/Bluemix_Cloudant NoSQL DB-sm_Credentials-1 private binding
actions
/sjfink@us.ibm.com_dev/Hello World                                private 
/sjfink@us.ibm.com_dev/My Action                                  private 
triggers
rules
MacBook-Pro-3:~ sjfink$ wsk action update A --sequence 'Hello World','Hello World'
usage: wsk [-h] [-v] [--apihost hostname] [--apiversion version]

           {action,activation,namespace,package,rule,trigger,sdk,property,list}
           ...
wsk: error: unrecognized arguments: Hello World,Hello World
MacBook-Pro-3:~ sjfink$ wsk action create A --sequence 'Hello World','Hello World'
ok: created action A
rabbah commented 8 years ago

Try --sequence before the action name on update.

sjfink commented 8 years ago

OK that worked. So I deduce that the semantics is in fact "update or create".

So the "bug" is that the 'create' and 'update' CLI syntax is inconsistent

vinodmut commented 8 years ago

We've seen this before. It's consistent if you do this:

wsk action create --sequence foo artifact
wsk action update --sequence foo artifact

Python argparse doesn't like --sequence after the action name for the update command because the artifact is an optional parameter (which is not the case for create). I'm not sure if there's a quick fix for this.

mbehrendt commented 8 years ago

is this going to be easier to solve once we've moved to the go cli?

vinodmut commented 8 years ago

Possibly. It'll depend on the argument parsing library the Go CLI uses.

sjfink commented 8 years ago

@mdeuser could you look into this?

mdeuser commented 8 years ago

@sjfink Sure. Yes, the command parsing package used by the Go CLI is a bit more forgiving regarding the argument flag positioning. Is the following syntax expected? The only restriction is that the CSV sequence set must be a single command line argument, so quote the whole CSV if there's a space in any of the the action names.

>wsk action create A --sequence "Hello World,Hello World"
ok: created action A

>wsk action update A --sequence "Hello World,Hello World"
ok: updated action A
sjfink commented 8 years ago

@mdeuser yes, I think that's the desired syntax. Thx.

mdeuser commented 8 years ago

@rabbah - Updating a non-existent action does not result in failure though. Is this expected?

>wsk action update doesnotexist --sequence "Hello World,Hello World"
ok: updated action doesnotexist

The PUT request URL is [PUT] https://openwhisk.ng.bluemix.net/api/v1/namespaces/mdeuser_dev/actions/doesnotexist?overwrite=true

rabbah commented 8 years ago

Semantics of update are create if it doesn't exist else update.

mdeuser commented 8 years ago

I think this behavior may lead to some confusion... a typo in the action name being updated. We can enforce this in the CLI if needed.

Validation of the existence of the supplied sequence action names being handled by #995

rabbah commented 8 years ago

The API method to create is a PUT. It is common for PUT to create a resource if it doesn't exist. We use ?overwrite true to explicitly create/override. I know from demos and use cases I've observed it's more convenient to use update than create with the deployment scripts.

-r

rabbah commented 8 years ago

@mdeuser I'm not sure I follow how or what "you're going to enforce it in the CLI if needed". wsk action create xxx vs wsk action create xxx --update? If the latter, we abandoned this long ago in favor of update as a first class verb wsk action update due to feedback.

Kick this back to you? Or is there anything specific you want me to act on?

mdeuser commented 8 years ago

@rabbah - If need be, the CLI can verify that the target action exists prior to issuing the update REST API call, leaving both wsk action create and wsk action update as distinct commands.

Another option would be just to update the help documentation. ie. change the current help from update an existing action to update an existing action; if action does not exist, create it.

rabbah commented 8 years ago

I suggest the latter - update the docs.

mdeuser commented 8 years ago

Issue #1151 opened to address the wsk CLI help change for all update commands.