Skip to content

Conversation

@ningyougang
Copy link
Contributor

@ningyougang ningyougang commented May 24, 2022

  • Support array result

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

  • Current is
    image
  • To-Be after the three prs merged
    image

activation get

  • Currrent is
    image
  • To-Be after the three prs merged
    image

}
}

if result, ok := response.([]interface{}); ok {
Copy link
Member

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.

Copy link
Contributor Author

@ningyougang ningyougang Aug 20, 2022

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.

Copy link
Member

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.

printBlockingTimeoutMsg(

Do we need to print the array result itself for the array case?

Copy link
Contributor Author

@ningyougang ningyougang Aug 22, 2022

Choose a reason for hiding this comment

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

The result is nil due to it is blocking with timeout, will not print the array result

I tested in such case (btw, i stopped all invokers containers)
image

The result is nil
image

Finally is
image

If i added sleep 61s for that action and make all invokers up again, the result is
image

For normally hello-array action with this pr, the result is
image

Copy link
Contributor Author

@ningyougang ningyougang Aug 22, 2022

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
image
it invoke use CLIActivationsOperations to get result in this pr
image

When build openwhisk pr, it would use RestActivationsOperations to get the result

Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Member

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?

Copy link
Contributor Author

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
@ningyougang ningyougang force-pushed the support-array-result branch from 4a5f897 to ae92c33 Compare August 21, 2022 04:23
@ningyougang
Copy link
Contributor Author

Closed as #517 merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants