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 2022/07/21 07:04:43 UTC

[GitHub] [openwhisk] ningyougang opened a new pull request, #5290: Support array result for common action and sequence action

ningyougang opened a new pull request, #5290:
URL: https://github.com/apache/openwhisk/pull/5290

   <!--- Provide a concise summary of your changes in the Title -->
   
   ## Description
   <!--- Provide a detailed description of your changes. -->
   <!--- Include details of what problem you are solving and how your changes are tested. -->
   POEM document pr: https://github.com/apache/openwhisk/pull/5244
   This feature is:  provide support array result feature for common action and sequence action. e.g.
   * For common action
   ```
   # cat ~/hello-array.js 
   function main(params) {
       console.log("-------------log test---------")
       return [{"key1":"value1"},{"key2":"value2"}];
   }
   
   # wsk -i action create hello-array-nodejs --kind nodejs:10 ~/hello-array.js 
   # wsk -i action invoke hello-array-nodejs -r -v
   REQUEST:
   [POST]	http://xxx.xxx.xxx.xxx:10001/api/v1/namespaces/_/actions/hello-array-nodejs?blocking=true&result=true
   ...
   ...
   Response body size is 37 bytes
   Response body received:
   [{"key1":"value1"},{"key2":"value2"}]
   ```
   * For sequence action
   ```
   # cat ~/split.js
   function main(msg) {
       var separator = msg.separator || /\r?\n/;
       var lines = payload.split(separator);
       return lines;
   }
   
   # cat ~/sort.js
   function main(msg) {
       var lines = msg || [];
       lines.sort();
       return lines;
   }
   
   # wsk -i action create /whisk.system/utils/split --kind nodejs:10 ~/split.js
   # wsk -i action create /whisk.system/utils/sort --kind nodejs:10 ~/sort.js
   # wsk -i action create mySequence --sequence /whisk.system/utils/split,/whisk.system/utils/sort
   # wsk -i action invoke --result mySequence --param payload "bbb\ncccc\naaaa"  -r -v
   REQUEST:
   [POST]	http://xxx.xxx.xxx.xxx:10001/api/v1/namespaces/_/actions/mySequence?blocking=true&result=true
   ...
   Response body size is 22 bytes
   Response body received:
   ["aaaa","bbbb","cccc"]
   ```
   * Activation check in elasticsearch
   ```
   # curl -u xxx:xxx -H "Content-Type: application/json" '[http://xxx.xxx.xxx.xxx:9200/lambda-exp_whisk.system/_search?pretty=true' -d  '{
     "query" : { "match" : { "activationId" : "9cc4dad8d81342fc84dad8d81302fc54" }},
     "from":0,
     "size":1
   }'
   {
     "took" : 3,
     ...
     "hits" : {
       "total" : 1,
       "max_score" : 5.1628795,
       "hits" : [
         {
           "_index" : "lambda-exp_whisk.system",
             ...
             "logs" : [
               "2022-07-21T06:53:16.543515915Z stdout: -------------log test---------"
             ],
             "name" : "hello-array-nodejs",
             ...
             "response" : {
               "result" : "[{\"key1\":\"value1\"},{\"key2\":\"value2\"}]",
               "size" : 37,
               "statusCode" : 0
             },
             ...
           }
         }
       ]
     }
   }
   
   # curl -u xxx:xxx -H "Content-Type: application/json" '[http://10.168.240.79:9200/lambda-exp_whisk.system/_search?pretty=true' -d  '{
     "query" : { "match" : { "activationId" : "9c774fe852de44cdb74fe852de94cd54" }},
     "from":0,
     "size":1
   }'
   ...
             "response" : {
               "result" : "[\"aaaa\",\"bbb\",\"cccc\"]",
               "size" : 21,
               "statusCode" : 0
             }
   ...
   ```
   The `response.result` must use `text` to store, because different activation use same filed may use different type, e.g.
   one activation's result's name filed value is string , e.g. {"name": "jack"}
   another activation's result's name filed value is int, e.g. {"name": 12}
   Elasticsearch doesn't support this, it store like this, it would report error.
   
   * Other works
   As above example, we already know, `nodejs` runtime supports this feature as well. 
   But other runtimes(e.g. go, java, php, etc) don't support,
   1. After this pr merged, there still has other works for runtime images, need to support subsequent prs for other images.
   2. And in CLI side, still has small mount works to parse the JSON array string to user.
   
   ## Related issue and scope
   <!--- Please include a link to a related issue if there is one. -->
   - [ ] I opened an issue to propose and discuss this change (#????)
   
   ## My changes affect the following components
   <!--- Select below all system components are affected by your change. -->
   <!--- Enter an `x` in all applicable boxes. -->
   - [ ] API
   - [x] Controller
   - [ ] Message Bus (e.g., Kafka)
   - [x] Loadbalancer
   - [ ] Scheduler
   - [x] Invoker
   - [ ] Intrinsic actions (e.g., sequences, conductors)
   - [ ] Data stores (e.g., CouchDB)
   - [x] Tests
   - [ ] Deployment
   - [ ] CLI
   - [ ] General tooling
   - [ ] Documentation
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
   - [ ] Bug fix (generally a non-breaking change which closes an issue).
   - [x] Enhancement or new feature (adds new functionality).
   - [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
   
   ## Checklist:
   <!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
   
   - [ ] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
   - [ ] I reviewed the [style guides](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md#coding-standards) and followed the recommendations (Travis CI will check :).
   - [ ] I added tests to cover my changes.
   - [ ] My changes require further changes to the documentation.
   - [ ] I updated the documentation where necessary.
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929691941


##########
core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/SequenceActions.scala:
##########
@@ -460,9 +465,10 @@ protected[actions] case class SequenceAccounting(atomicActionCnt: Int,
     // check conditions on payload that may lead to interrupting the execution of the sequence
     //     short-circuit the execution of the sequence iff the payload contains an error field
     //     and is the result of an action return, not the initial payload
-    val outputPayload = activation.response.result.map(_.asJsObject)
-    val payloadContent = outputPayload getOrElse JsObject.empty
-    val errorField = payloadContent.fields.get(ActivationResponse.ERROR_FIELD)
+    val errorField: Option[JsValue] = activation.response.result match {
+      case Some(JsObject(fields)) => fields.get(ActivationResponse.ERROR_FIELD)
+      case _                      => None

Review Comment:
   * So users can't define the explicit error response when the result is a JSON array.
   Yes, can't define the explicit error response for a JSON array,
   but if we want to define the explicit error response, normally, user should return the JSON-object result.
   * But there would be no difference in the JSON-object-based action, right?
   For the JSON-object-based action, have no any bad influences on these actions, have no any change.
   * Regarding
   ```
   Sometimes, libraries return an error filed and OW considers that as an error of an action as well.
   Since there would be no behavioral difference in the JSON-object-based actions, there would be no difference in existing semantics, right?
   ```
   I am not sure whether i understand correctly, if libraries return an `error` or user's codes has some error, normally, there would has `catch(....)` statement in user's code, and in catch statement, user can return the error or some excetion error with JSON-object result.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926330721


##########
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala:
##########
@@ -57,7 +57,7 @@ case class ActivationMessage(override val transid: TransactionId,
                              activationId: ActivationId,
                              rootControllerIndex: ControllerInstanceId,
                              blocking: Boolean,
-                             content: Option[JsObject],
+                             content: Option[JsValue],

Review Comment:
   For common action to pass parameter as dict, it would use `Option[JsObject]`
   For sequence action to pass parameter as array, it would use `Option[JsArray]`, e.g. the first action's JsArray result as the next action's input param.



##########
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala:
##########
@@ -57,7 +57,7 @@ case class ActivationMessage(override val transid: TransactionId,
                              activationId: ActivationId,
                              rootControllerIndex: ControllerInstanceId,
                              blocking: Boolean,
-                             content: Option[JsObject],
+                             content: Option[JsValue],

Review Comment:
   * For common action to pass parameter as dict, it would use `Option[JsObject]`
   * For sequence action to pass parameter as array, it would use `Option[JsArray]`, e.g. the first action's JsArray result as the next action's input param.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926350523


##########
tests/src/test/scala/actionContainers/ActionContainer.scala:
##########
@@ -52,6 +52,7 @@ import org.apache.openwhisk.core.containerpool.Container
 trait ActionContainer {
   def init(value: JsValue): (Int, Option[JsObject])
   def run(value: JsValue): (Int, Option[JsObject])
+  def runForJsArray(value: JsValue): (Int, Option[JsArray])

Review Comment:
   Normally, need to change `def run(...)` as below
   ```
   def run(value: JsValue): (Int, Option[JsValue])    // Option[JsObject] -> Option[JsValue]
   ```
   But i just add another new method here, two reasons.
   * If change `def run` directly, there has a lot of changes.
   * It is just for test, so i just add another new method, there has no need to introduce too many code modifications



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926350523


##########
tests/src/test/scala/actionContainers/ActionContainer.scala:
##########
@@ -52,6 +52,7 @@ import org.apache.openwhisk.core.containerpool.Container
 trait ActionContainer {
   def init(value: JsValue): (Int, Option[JsObject])
   def run(value: JsValue): (Int, Option[JsObject])
+  def runForJsArray(value: JsValue): (Int, Option[JsArray])

Review Comment:
   Normally, need to change `def run(...)` as below
   ```
   def run(value: JsValue): (Int, Option[JsValue])    // Option[JsObject] -> Option[JsValue]
   ```
   But i just add another new method here, two reasons.
   * If change `def run` directly, there has a lot of code changes.
   * It is just for test, so i just add another new method, there has no need to introduce too many code modifications



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926330721


##########
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala:
##########
@@ -57,7 +57,7 @@ case class ActivationMessage(override val transid: TransactionId,
                              activationId: ActivationId,
                              rootControllerIndex: ControllerInstanceId,
                              blocking: Boolean,
-                             content: Option[JsObject],
+                             content: Option[JsValue],

Review Comment:
   For common action to pass parameter as dict, it would use `Option[JsObject]`
   For sequence action to pass parameter as array, it would use `Option[JsArray]`



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] codecov-commenter commented on pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#issuecomment-1191177458

   # [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5290?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#5290](https://codecov.io/gh/apache/openwhisk/pull/5290?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (460fdff) into [master](https://codecov.io/gh/apache/openwhisk/commit/a03507ceeb2c4648c875ed68273de1448ec0d074?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a03507c) will **decrease** coverage by `75.34%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #5290       +/-   ##
   ==========================================
   - Coverage   79.90%   4.56%   -75.35%     
   ==========================================
     Files         238     238               
     Lines       14161   14184       +23     
     Branches      603     586       -17     
   ==========================================
   - Hits        11316     648    -10668     
   - Misses       2845   13536    +10691     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/openwhisk/core/connector/Message.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29ubmVjdG9yL01lc3NhZ2Uuc2NhbGE=) | `15.38% <0.00%> (-60.91%)` | :arrow_down: |
   | [...whisk/core/containerpool/AkkaContainerClient.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Ba2thQ29udGFpbmVyQ2xpZW50LnNjYWxh) | `0.00% <0.00%> (-78.73%)` | :arrow_down: |
   | [...pache/openwhisk/core/containerpool/Container.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC9Db250YWluZXIuc2NhbGE=) | `2.15% <ø> (-88.18%)` | :arrow_down: |
   | [...e/elasticsearch/ElasticSearchActivationStore.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZGF0YWJhc2UvZWxhc3RpY3NlYXJjaC9FbGFzdGljU2VhcmNoQWN0aXZhdGlvblN0b3JlLnNjYWxh) | `0.00% <0.00%> (-77.91%)` | :arrow_down: |
   | [...pache/openwhisk/core/entity/ActivationResult.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvZW50aXR5L0FjdGl2YXRpb25SZXN1bHQuc2NhbGE=) | `0.00% <0.00%> (-90.67%)` | :arrow_down: |
   | [...cala/org/apache/openwhisk/http/ErrorResponse.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NjYWxhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2h0dHAvRXJyb3JSZXNwb25zZS5zY2FsYQ==) | `44.55% <0.00%> (-44.56%)` | :arrow_down: |
   | [...org/apache/openwhisk/core/controller/Actions.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9BY3Rpb25zLnNjYWxh) | `0.00% <0.00%> (-90.79%)` | :arrow_down: |
   | [...core/controller/actions/PostActionActivation.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9hY3Rpb25zL1Bvc3RBY3Rpb25BY3RpdmF0aW9uLnNjYWxh) | `0.00% <ø> (-100.00%)` | :arrow_down: |
   | [...isk/core/controller/actions/PrimitiveActions.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9hY3Rpb25zL1ByaW1pdGl2ZUFjdGlvbnMuc2NhbGE=) | `0.00% <0.00%> (-86.42%)` | :arrow_down: |
   | [...hisk/core/controller/actions/SequenceActions.scala](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9jb250cm9sbGVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udHJvbGxlci9hY3Rpb25zL1NlcXVlbmNlQWN0aW9ucy5zY2FsYQ==) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
   | ... and [213 more](https://codecov.io/gh/apache/openwhisk/pull/5290/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929706322


##########
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala:
##########
@@ -1082,25 +1082,28 @@ object ContainerProxy {
    * @param initArgs set of parameters to treat as initialization arguments
    * @return A partition of the arguments into an environment variables map and the JsObject argument to the action
    */
-  def partitionArguments(content: Option[JsObject], initArgs: Set[String]): (Map[String, JsValue], JsObject) = {
+  def partitionArguments(content: Option[JsValue], initArgs: Set[String]): (Map[String, JsValue], JsValue) = {
     content match {
-      case None                         => (Map.empty, JsObject.empty)
-      case Some(js) if initArgs.isEmpty => (Map.empty, js)
-      case Some(js) =>
-        val (env, args) = js.fields.partition(k => initArgs.contains(k._1))
+      case None                                       => (Map.empty, JsObject.empty)
+      case Some(JsObject(fields)) if initArgs.isEmpty => (Map.empty, JsObject(fields))
+      case Some(JsObject(fields)) =>
+        val (env, args) = JsObject(fields).fields.partition(k => initArgs.contains(k._1))
         (env, JsObject(args))
+      case Some(JsArray(elements)) => (Map.empty, JsArray(elements))

Review Comment:
   No, would report error, but there has a optimize point here.
   Change `JsObject(fields).fields` to `fields`



##########
tests/src/test/scala/system/basic/WskSequenceTests.scala:
##########
@@ -541,6 +541,31 @@ class WskSequenceTests extends TestHelpers with WskTestHelpers with StreamLoggin
       }
   }
 
+  it should "invoke a sequence which support array result" in withAssetCleaner(wskprops) { (wp, assetHelper) =>

Review Comment:
   Updated accordingly



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#issuecomment-1197550051

   > It generally looks good to me. So it is not a breaking change and there would be no issue with the existing features, right? Do we have to merge this before merging all runtime changes?
   
   Yes, has no issue.
   
   Should merge this first, because other runtime prs depend on this pr.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] style95 commented on pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
style95 commented on PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#issuecomment-1197548869

   It generally looks good to me.
   So it is not a breaking change and there would be no issue with the existing features, right?
   Do we have to merge this before merging all runtime changes?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#issuecomment-1197961214

   Have any comment, due to there exist several runtime pr depend on this pr, so i just asked.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929682591


##########
common/scala/src/main/scala/org/apache/openwhisk/core/entity/ActivationResult.scala:
##########
@@ -203,7 +203,7 @@ protected[core] object ActivationResponse extends DefaultJsonProtocol {
         truncated match {
           case None =>
             val sizeOpt = Option(str).map(_.length)
-            Try { str.parseJson.asJsObject } match {
+            Try { str.parseJson } match {
               case scala.util.Success(result @ JsObject(fields)) =>

Review Comment:
   * If without this pr, i think we can remove `.asJsObject`.
   * But with this pr, we need to support JsArray, must remove `.asJsObject` to match below codes
   ![image](https://user-images.githubusercontent.com/11749867/180961843-6584f818-91bc-4c53-9ac8-ef8d26e73530.png)
   



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926335947


##########
tests/src/test/scala/system/basic/WskActionTests.scala:
##########
@@ -392,4 +392,18 @@ class WskActionTests extends TestHelpers with WskTestHelpers with JsHelpers with
       }
   }
 
+  it should "invoke an action with a array result" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
+    val name = "helloArray"
+    assetHelper.withCleaner(wsk.action, name) { (action, _) =>
+      action.create(name, Some(TestUtils.getTestActionFilename("helloArray.js")))
+    }
+
+    val run = wsk.action.invoke(name)
+    withActivation(wsk.activation, run) { activation =>
+      activation.response.status shouldBe "success"
+      activation.response.result shouldBe Some(
+        JsArray(JsObject("key1" -> JsString("value1")), JsObject("key2" -> JsString("value2"))))
+    }
+  }

Review Comment:
   Because `nodejs runtime` suports array result by default, add a system test case for return array result for it to test controller/invoker whether works 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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926333658


##########
common/scala/src/main/scala/org/apache/openwhisk/core/database/elasticsearch/ElasticSearchActivationStore.scala:
##########
@@ -412,7 +412,10 @@ class ElasticSearchActivationStore(
             .get("result")
             .map { r =>
               val JsString(data) = r
-              data.parseJson.asJsObject
+              data.parseJson match {
+                case JsArray(elements) => JsArray(elements)

Review Comment:
   Support `store the JsArray result to ElasticSearch`.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929675790


##########
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/AkkaContainerClient.scala:
##########
@@ -201,6 +201,19 @@ object AkkaContainerClient {
     result
   }
 
+  /** A helper method to post one single request to a connection. Used for container tests. */
+  def postForJsArray(host: String, port: Int, endPoint: String, content: JsValue, timeout: FiniteDuration)(
+    implicit logging: Logging,
+    as: ActorSystem,
+    ec: ExecutionContext,
+    tid: TransactionId): (Int, Option[JsArray]) = {

Review Comment:
   * `def runForJsArray` is added for test case, the return value is `(Int, Option[JsArray])`
   * call chain here is: `def runForJsArray` -> `private def syncPostForJsArray` -> `def postForJsArray`
   
   Actually, in order to support test, we can change below method to support return `(Int, Option[JsValue])` also
   ![image](https://user-images.githubusercontent.com/11749867/180959848-4d0317a2-264a-4ba5-806b-cf179b5e4baa.png)
   But if we change the run directly, there would be a lot of changes in openwhisk repo and all runtime codes should change as well.
   
   So here, i added another extra method to test return array
   ![image](https://user-images.githubusercontent.com/11749867/180960350-2b90fa00-39af-438e-97d7-d63bb86390dd.png)
   This is just for `impact the original code as little as possible`: https://github.com/apache/openwhisk/pull/5290#discussion_r926350523



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926335348


##########
tests/dat/actions/split-array.js:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Splits a string into an array of strings
+ * Return lines in an array.
+ * @param payload A string.
+ * @param separator The character, or the regular expression, to use for splitting the string
+ */
+function main(msg) {
+    var separator = msg.separator || /\r?\n/;
+    var payload = msg.payload.toString();
+    var lines = payload.split(separator);
+    // return array as next action's input
+    return lines;

Review Comment:
   As comment said, return array as next action's input
   This script file is used as test case for invoke sequence action which support array result



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929675790


##########
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/AkkaContainerClient.scala:
##########
@@ -201,6 +201,19 @@ object AkkaContainerClient {
     result
   }
 
+  /** A helper method to post one single request to a connection. Used for container tests. */
+  def postForJsArray(host: String, port: Int, endPoint: String, content: JsValue, timeout: FiniteDuration)(
+    implicit logging: Logging,
+    as: ActorSystem,
+    ec: ExecutionContext,
+    tid: TransactionId): (Int, Option[JsArray]) = {

Review Comment:
   * `def runForJsArray` is added for test case, the return value is `(Int, Option[JsArray])`
   * call chain here is: `def runForJsArray` -> `private def syncPostForJsArray` -> `def postForJsArray`
   
   Actually, in order to support test, we can change below method to support return `(Int, Option[JsValue])` also
   ![image](https://user-images.githubusercontent.com/11749867/180959848-4d0317a2-264a-4ba5-806b-cf179b5e4baa.png)
   But if we change the run directly, there would be a lot of changes in openwhisk repo and all runtime codes should change as well.
   
   So here, i added another extra method to test return array
   ![image](https://user-images.githubusercontent.com/11749867/180960350-2b90fa00-39af-438e-97d7-d63bb86390dd.png)
   This is just for `impact the original code as little as possible`



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929691941


##########
core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/SequenceActions.scala:
##########
@@ -460,9 +465,10 @@ protected[actions] case class SequenceAccounting(atomicActionCnt: Int,
     // check conditions on payload that may lead to interrupting the execution of the sequence
     //     short-circuit the execution of the sequence iff the payload contains an error field
     //     and is the result of an action return, not the initial payload
-    val outputPayload = activation.response.result.map(_.asJsObject)
-    val payloadContent = outputPayload getOrElse JsObject.empty
-    val errorField = payloadContent.fields.get(ActivationResponse.ERROR_FIELD)
+    val errorField: Option[JsValue] = activation.response.result match {
+      case Some(JsObject(fields)) => fields.get(ActivationResponse.ERROR_FIELD)
+      case _                      => None

Review Comment:
   * So users can't define the explicit error response when the result is a JSON array.
   Yes, can't define the explicit error response for a JSON array,
   but if we want to define the explicit error response, normally, user should return the JSON-object result.
   * But there would be no difference in the JSON-object-based action, right?
   For the JSON-object-based action, have no any bad influences on these actions, have no any change.
   * Regarding
   ```
   Sometimes, libraries return an error filed and OW considers that as an error of an action as well.
   Since there would be no behavioral difference in the JSON-object-based actions, there would be no difference in existing semantics, right?
   ```
   I am not sure whether i understand correctly, if libraries return an `error` or user's codes has some error, normally, there would has `catch(....)` statement in user's code, and in catch statement, user can return the error or some excetion error with JSON-object result.
   If user codes run well without any error or exception, can return array result finally.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929694176


##########
tests/src/test/scala/org/apache/openwhisk/core/controller/test/ConductorsApiTests.scala:
##########
@@ -345,26 +351,39 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
             case `echo` => // echo action
               Future(Right(respond(action, msg, args)))
             case `conductor` => // see tests/dat/actions/conductor.js
-              val result =
-                if (args.fields.get("error") isDefined) args
-                else {
-                  val action = args.fields.get("action") map { action =>
-                    Map("action" -> action)
-                  } getOrElse Map.empty
-                  val state = args.fields.get("state") map { state =>
-                    Map("state" -> state)
-                  } getOrElse Map.empty
-                  val wrappedParams = args.fields.getOrElse("params", JsObject.empty).asJsObject.fields
-                  val escapedParams = args.fields - "action" - "state" - "params"
-                  val params = Map("params" -> JsObject(wrappedParams ++ escapedParams))
-                  JsObject(params ++ action ++ state)
+              val result = {
+                args match {
+                  case JsObject(fields) =>
+                    if (fields.get("error") isDefined) args
+                    else {
+                      val action = fields.get("action") map { action =>
+                        Map("action" -> action)
+                      } getOrElse Map.empty
+                      val state = fields.get("state") map { state =>
+                        Map("state" -> state)
+                      } getOrElse Map.empty
+                      val wrappedParams = fields.getOrElse("params", JsObject.empty).asJsObject.fields
+                      val escapedParams = fields - "action" - "state" - "params"
+                      val params = Map("params" -> JsObject(wrappedParams ++ escapedParams))
+                      JsObject(params ++ action ++ state)
+                    }
+                  case _ => JsObject.empty

Review Comment:
   hm..i am not sure, if want to support, i think we can submit in next pr.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] style95 commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
style95 commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929523068


##########
common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/AkkaContainerClient.scala:
##########
@@ -201,6 +201,19 @@ object AkkaContainerClient {
     result
   }
 
+  /** A helper method to post one single request to a connection. Used for container tests. */
+  def postForJsArray(host: String, port: Int, endPoint: String, content: JsValue, timeout: FiniteDuration)(
+    implicit logging: Logging,
+    as: ActorSystem,
+    ec: ExecutionContext,
+    tid: TransactionId): (Int, Option[JsArray]) = {

Review Comment:
   Is there any reason that we can't just change the result type to `(Int, Option[JsValue])`?



##########
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala:
##########
@@ -380,9 +380,13 @@ object Activation extends DefaultJsonProtocol {
 
   /** Get "StatusCode" from result response set by action developer * */
   def userDefinedStatusCode(result: Option[JsValue]): Option[Int] = {
-    val statusCode = JsHelpers
-      .getFieldPath(result.get.asJsObject, ERROR_FIELD, "statusCode")
-      .orElse(JsHelpers.getFieldPath(result.get.asJsObject, "statusCode"))
+    val statusCode: Option[JsValue] = result match {
+      case Some(JsObject(fields)) =>
+        JsHelpers
+          .getFieldPath(JsObject(fields), ERROR_FIELD, "statusCode")
+          .orElse(JsHelpers.getFieldPath(JsObject(fields), "statusCode"))
+      case _ => None

Review Comment:
   This is for the array result case and there could be no `statusCode` field in the array result, right?



##########
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala:
##########
@@ -1082,25 +1082,28 @@ object ContainerProxy {
    * @param initArgs set of parameters to treat as initialization arguments
    * @return A partition of the arguments into an environment variables map and the JsObject argument to the action
    */
-  def partitionArguments(content: Option[JsObject], initArgs: Set[String]): (Map[String, JsValue], JsObject) = {
+  def partitionArguments(content: Option[JsValue], initArgs: Set[String]): (Map[String, JsValue], JsValue) = {
     content match {
-      case None                         => (Map.empty, JsObject.empty)
-      case Some(js) if initArgs.isEmpty => (Map.empty, js)
-      case Some(js) =>
-        val (env, args) = js.fields.partition(k => initArgs.contains(k._1))
+      case None                                       => (Map.empty, JsObject.empty)
+      case Some(JsObject(fields)) if initArgs.isEmpty => (Map.empty, JsObject(fields))
+      case Some(JsObject(fields)) =>
+        val (env, args) = JsObject(fields).fields.partition(k => initArgs.contains(k._1))
         (env, JsObject(args))
+      case Some(JsArray(elements)) => (Map.empty, JsArray(elements))

Review Comment:
   ```suggestion
         case None                                       => (Map.empty, JsObject.empty)
         case Some(JsArray(elements)) => (Map.empty, JsArray(elements))
         case Some(fields) if initArgs.isEmpty => (Map.empty, fields)
         case Some(fields) =>
           val (env, args) = fields.fields.partition(k => initArgs.contains(k._1))
           (env, JsObject(args))
   
   ```
   
   This one can be changed like this?



##########
tests/src/test/scala/system/basic/WskSequenceTests.scala:
##########
@@ -541,6 +541,31 @@ class WskSequenceTests extends TestHelpers with WskTestHelpers with StreamLoggin
       }
   }
 
+  it should "invoke a sequence which support array result" in withAssetCleaner(wskprops) { (wp, assetHelper) =>

Review Comment:
   ```suggestion
     it should "invoke a sequence which supports array result" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
   ```



##########
tests/src/test/scala/org/apache/openwhisk/core/controller/test/ConductorsApiTests.scala:
##########
@@ -345,26 +351,39 @@ class ConductorsApiTests extends ControllerTestCommon with WhiskActionsApi {
             case `echo` => // echo action
               Future(Right(respond(action, msg, args)))
             case `conductor` => // see tests/dat/actions/conductor.js
-              val result =
-                if (args.fields.get("error") isDefined) args
-                else {
-                  val action = args.fields.get("action") map { action =>
-                    Map("action" -> action)
-                  } getOrElse Map.empty
-                  val state = args.fields.get("state") map { state =>
-                    Map("state" -> state)
-                  } getOrElse Map.empty
-                  val wrappedParams = args.fields.getOrElse("params", JsObject.empty).asJsObject.fields
-                  val escapedParams = args.fields - "action" - "state" - "params"
-                  val params = Map("params" -> JsObject(wrappedParams ++ escapedParams))
-                  JsObject(params ++ action ++ state)
+              val result = {
+                args match {
+                  case JsObject(fields) =>
+                    if (fields.get("error") isDefined) args
+                    else {
+                      val action = fields.get("action") map { action =>
+                        Map("action" -> action)
+                      } getOrElse Map.empty
+                      val state = fields.get("state") map { state =>
+                        Map("state" -> state)
+                      } getOrElse Map.empty
+                      val wrappedParams = fields.getOrElse("params", JsObject.empty).asJsObject.fields
+                      val escapedParams = fields - "action" - "state" - "params"
+                      val params = Map("params" -> JsObject(wrappedParams ++ escapedParams))
+                      JsObject(params ++ action ++ state)
+                    }
+                  case _ => JsObject.empty

Review Comment:
   If we just do this, can we support conductor actions with JSON array results?



##########
common/scala/src/main/scala/org/apache/openwhisk/core/entity/ActivationResult.scala:
##########
@@ -203,7 +203,7 @@ protected[core] object ActivationResponse extends DefaultJsonProtocol {
         truncated match {
           case None =>
             val sizeOpt = Option(str).map(_.length)
-            Try { str.parseJson.asJsObject } match {
+            Try { str.parseJson } match {
               case scala.util.Success(result @ JsObject(fields)) =>

Review Comment:
   So even without `asJsObject`, the result would match here?
   Then, it was not necessary in the first place?



##########
core/controller/src/main/scala/org/apache/openwhisk/core/controller/actions/SequenceActions.scala:
##########
@@ -460,9 +465,10 @@ protected[actions] case class SequenceAccounting(atomicActionCnt: Int,
     // check conditions on payload that may lead to interrupting the execution of the sequence
     //     short-circuit the execution of the sequence iff the payload contains an error field
     //     and is the result of an action return, not the initial payload
-    val outputPayload = activation.response.result.map(_.asJsObject)
-    val payloadContent = outputPayload getOrElse JsObject.empty
-    val errorField = payloadContent.fields.get(ActivationResponse.ERROR_FIELD)
+    val errorField: Option[JsValue] = activation.response.result match {
+      case Some(JsObject(fields)) => fields.get(ActivationResponse.ERROR_FIELD)
+      case _                      => None

Review Comment:
   So users can't define the explicit error response when the result is a JSON array.
   But there would be no difference in the JSON-object-based action, right?
   
   Sometimes, libraries return an `error` filed and OW considers that as an error of an action as well.
   Since there would be no behavioral difference in the JSON-object-based actions, there would be no difference in existing semantics, right?



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r929664722


##########
common/scala/src/main/scala/org/apache/openwhisk/core/connector/Message.scala:
##########
@@ -380,9 +380,13 @@ object Activation extends DefaultJsonProtocol {
 
   /** Get "StatusCode" from result response set by action developer * */
   def userDefinedStatusCode(result: Option[JsValue]): Option[Int] = {
-    val statusCode = JsHelpers
-      .getFieldPath(result.get.asJsObject, ERROR_FIELD, "statusCode")
-      .orElse(JsHelpers.getFieldPath(result.get.asJsObject, "statusCode"))
+    val statusCode: Option[JsValue] = result match {
+      case Some(JsObject(fields)) =>
+        JsHelpers
+          .getFieldPath(JsObject(fields), ERROR_FIELD, "statusCode")
+          .orElse(JsHelpers.getFieldPath(JsObject(fields), "statusCode"))
+      case _ => None

Review Comment:
   Yes, for array result, seems can't get the userDefined statusCode



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang commented on a diff in pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290#discussion_r926350523


##########
tests/src/test/scala/actionContainers/ActionContainer.scala:
##########
@@ -52,6 +52,7 @@ import org.apache.openwhisk.core.containerpool.Container
 trait ActionContainer {
   def init(value: JsValue): (Int, Option[JsObject])
   def run(value: JsValue): (Int, Option[JsObject])
+  def runForJsArray(value: JsValue): (Int, Option[JsArray])

Review Comment:
   Normally, need to change `def run(...)` as below
   ```
   def run(value: JsValue): (Int, Option[JsValue])
   ```
   But i just add another new method here, two reasons.
   * If change `def run` directly, there has a lot of changes.
   * It is just for test, so i just add another new method, there has no need to introduce too many code modifications



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [openwhisk] ningyougang merged pull request #5290: Support array result for common action and sequence action

Posted by GitBox <gi...@apache.org>.
ningyougang merged PR #5290:
URL: https://github.com/apache/openwhisk/pull/5290


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org