Support array result include sequence action by ningyougang · Pull Request #170 · apache/openwhisk-runtime-go

ningyougang · 2022-07-25T02:24:02Z

Support array result
- make go runtime to support array result for common action
- make go runtime to support array result for sequence action (support array as input param)

ningyougang · 2022-07-25T06:00:14Z

The steps of test Support sequence action

Write split.go and sort.go

[root@nccddev130026 ~]# cat ~/split.go 
package main

import (
	"strings"
)

// Main is the function implementing the action
func Main(obj map[string]interface{}) []interface{} {
	payload, ok := obj["payload"].(string)
	if !ok {
		payload = "aaaa\nbbbb\ncccc"
	}

	separator, ok := obj["separator"].(string)
	if !ok {
		separator = "\n"
	}
	stringSlice := strings.Split(payload, separator)
	output := make([]interface{}, len(stringSlice))
	for i, v := range stringSlice {
		output[i] = v
	}
    return output
}

[root@nccddev130026 ~]# cat ~/sort.go 
package main

import (
	"fmt"
	"sort"
)

// Main is the function implementing the action
func Main(event []interface{}) []interface{} {
	input := make([]string, len(event))
	for i, v := range event {
		input[i] = fmt.Sprint(v)
	}
	sort.Strings(input)
	output := make([]interface{}, len(event))
	for i, v := range input {
		output[i] = v
	}
	return output
}

Create sequence action and invoke it (go1.17 and go1.18 both tested, worked well)

wsk -i action create /whisk.system/utils/split-go --kind go:1.17 ~/split.go
wsk -i action create /whisk.system/utils/sort-go --kind go:1.17 ~/sort.go
wsk -i action create mySequence-go --sequence /whisk.system/utils/split-go,/whisk.system/utils/sort-go
wsk -i action invoke --result mySequence-go --param payload "dddd\nbbbb\ncccc" -r -v

ningyougang · 2022-08-03T03:28:14Z

tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala

+      c.init(initPayload(src))._1 shouldBe (200)
+      val result = c.runForJsArray(JsObject())
+      result._1 shouldBe (200)
+      result._2 shouldBe Some(JsArray(JsString("a"), JsString("b")))


Compare the result

ningyougang · 2022-08-03T23:40:49Z

openwhisk/runHandler.go

 	if err != nil {
-		sendError(w, http.StatusBadGateway, "The action did not return a dictionary or array.")
-		return
+		err = json.Unmarshal(response, &objarray)


First, parse the result as json, if failed, parse the result as array.

ningyougang · 2022-08-03T23:42:05Z

golang1.18/lib/launcher.go

-		var payload map[string]interface{}
+		isJsonObjectParam := true
+		var payloadForJsonObject map[string]interface{}
+		var payloadForJsonArray []interface{}


Support jsonArray as input param as well. this is for sequence action

ningyougang · 2022-08-03T23:42:52Z

golang1.17/lib/launcher.go

-		var payload map[string]interface{}
+		isJsonObjectParam := true
+		var payloadForJsonObject map[string]interface{}
+		var payloadForJsonArray []interface{}


Support json array as input param, this is for sequence action

ningyougang · 2022-08-03T23:45:24Z

@style95 @upgle ,welcome review, due to other runtimes(e.g. java) depends on go runtime (use go runtime's /bin/proxy),
need to merge this pr firstly.

style95 · 2022-08-03T23:57:41Z

tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala

+      s"""
+         |package main
+         |
+         |func Main(obj map[string]interface{}) []interface{} {


So if we want to use the JSON array params, do we have to use this signature for the main function?

for JSON array params, can use like blow

// Main is the function implementing the action func Main(event []interface{}) []interface{} { input := make([]string, len(event)) for i, v := range event { input[i] = fmt.Sprint(v) } sort.Strings(input) output := make([]interface{}, len(event)) for i, v := range input { output[i] = v } return output }

hm..it is better to add a test case for json array params as well.

ok now I get it.

So originally the signature of the Main method is

func Main(obj map[string]interface{}) map[string]interface{} { ... }

And now we add support for JSON array params.

style95 · 2022-08-04T00:00:07Z

common/gobuild.py.launcher.go

 		// process the request
-		result := action(payload)
+		var result interface{}
+		funcMain := reflect.ValueOf(Main)


I am not that familiar with the runtime-go code.
Where this Main comes from?

It is the user's action's Main. i debugged the /init(or /run) before, when execute /init, it will inject user action codes to action container's suitable directory, the common/gobuild.py.launcher.go can recognize this Main method.

ningyougang · 2022-08-04T00:42:37Z

common/gobuild.py.launcher.go

+		funcMain := reflect.ValueOf(Main)
+		if isJsonObjectParam {
+			param := []reflect.Value{reflect.ValueOf(payloadForJsonObject)}
+			reflectResult := funcMain.Call(param)


If invokes Main directly, compile would be failed.
So i used go reflect to invoke Main

style95

I am ok with this change as long as we add this test case.
https://github.com/apache/openwhisk-runtime-go/pull/170/files#r937249726

style95 · 2022-08-04T01:50:58Z

tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala

+      s"""
+         |package main
+         |
+         |func Main(obj map[string]interface{}) []interface{} {


ok now I get it.

So originally the signature of the Main method is

func Main(obj map[string]interface{}) map[string]interface{} { ... }

And now we add support for JSON array params.

ningyougang · 2022-08-04T02:07:15Z

tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala

+    }
+  }
+
+  it should "support array as input param" in {


Add another test case for support array as input param

ningyougang · 2022-08-04T02:07:57Z

@style95 , new test case is added here: https://github.com/apache/openwhisk-runtime-go/pull/170/files#diff-37f21cca37a0702d947ad4948d51808b04eae395f1deb7b3d2d8d51cdef61723R163

upgle · 2022-08-04T03:34:46Z

tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala

+      s"""
+         |package main
+         |
+         |func Main(obj []interface{}) []interface{} {


I wonder what happens when the argument of the main function is an array and an object parameter is passed.

@upgle
Use reflect to invoke the Main, e.g.

pass object parameter

https://github.com/apache/openwhisk-runtime-go/pull/170/files#diff-8f7cd20b9f7340baadcd155468f78cad8ba149c8d97c6e068a830daf1c210320R110

pass array parameter

https://github.com/apache/openwhisk-runtime-go/pull/170/files#diff-8f7cd20b9f7340baadcd155468f78cad8ba149c8d97c6e068a830daf1c210320R114

Can't invoke the Main directly due to compile error, should use reflect to invoke.

Btw, why can invoke Main direclty before? because the input param's data type is explicit.
But in this pr, the data type supports 2 data types, so should use reflect to invoke it.

ningyougang · 2022-08-05T00:06:47Z

I will merge due to other runtimes depend on this pr, and because exist 2 test cases，i think it is safe to mrege it
If has issue, open new pr.

ningyougang force-pushed the support-array-result-include-sequence-action branch from 7b7f3e8 to a54a9ba Compare August 3, 2022 03:06

ningyougang mentioned this pull request Aug 3, 2022

Upgrade gradle version #172

Merged

1 task

ningyougang commented Aug 3, 2022

View reviewed changes

ningyougang mentioned this pull request Aug 3, 2022

[DON'T MERGE] check current master's travis ci whether pass #171

Closed

ningyougang added 6 commits August 3, 2022 17:06

Make /run to return array result

881e291

Make go1.17 to return array result

5d9d514

Make go1.18 to support array result

820ece8

Make common launcher.go to support array result

f404472

Add test case

ad69b32

Optimize the test result

2243586

ningyougang force-pushed the support-array-result-include-sequence-action branch from a54a9ba to 2243586 Compare August 3, 2022 09:10

ningyougang commented Aug 3, 2022

View reviewed changes

style95 reviewed Aug 3, 2022

View reviewed changes

style95 reviewed Aug 4, 2022

View reviewed changes

ningyougang commented Aug 4, 2022

View reviewed changes

style95 approved these changes Aug 4, 2022

View reviewed changes

Add test case for support array as input param

f077a0a

ningyougang commented Aug 4, 2022

View reviewed changes

upgle reviewed Aug 4, 2022

View reviewed changes

ningyougang mentioned this pull request Aug 4, 2022

Support array result apache/openwhisk-cli#516

Closed

1 task

ningyougang merged commit 6696cee into apache:master Aug 5, 2022

ningyougang mentioned this pull request Aug 5, 2022

Add document for support array result #173

Merged

1 task

Conversation

ningyougang commented Jul 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ningyougang commented Jul 25, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ningyougang Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ningyougang commented Aug 3, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

style95 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ningyougang commented Aug 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ningyougang commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ningyougang commented Jul 25, 2022 •

edited

Loading

ningyougang Aug 3, 2022 •

edited

Loading

ningyougang commented Aug 5, 2022 •

edited

Loading