-
Notifications
You must be signed in to change notification settings - Fork 97
Support array result #516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support array result #516
Conversation
| } | ||
| } | ||
|
|
||
| if result, ok := response.([]interface{}); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we insist that field be "" if response is an array? I'm not sure exactly how this function is used, but it might be error prone to just silently return the array result when the caller asked for a specific field of what they must have been expecting to be a JSON object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgrove-oss yes, you right, need to return "" when it is array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ningyougang
This method was used to print the invocation result.
openwhisk-cli/commands/action.go
Line 238 in 92c251f
| printBlockingTimeoutMsg( |
Do we need to print the array result itself for the array case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, i already cherry-pick this pr's commits to #517,
Because add-array-result pr: apache/openwhisk#5290 is merged before than this pr, so when build this pr and execute this pr's test cases, it would execute openwhisk core relative's add-array-result feature test cases. e.g. when invoke below test case

it invoke use CLIActivationsOperations to get result in this pr

When build openwhisk pr, it would use RestActivationsOperations to get the result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, it seems now I get it.
Previously, it returns var res interface{} without any initialization, when it failed to find the field.
Is this an empty string in case it could not find anything?
Isn't it nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would return emtry string now if it failed to find the field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it an empty string or nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nil
If return array, make the result return empty string
00fa4ce to
443499c
Compare
4a5f897 to
ae92c33
Compare
|
Closed as #517 merged |





Another brother prs:
apache/openwhisk-client-go#153 (should be merged secondly)
apache/openwhisk-wskdeploy#1153 (should be merged firslty)
This pr can be merged thirdly
After merged this three prs and if openwhisk itself support
support array result:use wsk can get the correct arrary result. e.g
invoke action
activation get