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/25 02:24:04 UTC
[GitHub] [openwhisk-runtime-go] ningyougang opened a new pull request, #170: Support array result include sequence action
ningyougang opened a new pull request, #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170
- [x] Make /run to support array result
- [x] Make go1.17/1.18 to support array result as input param
- [x] Add test case
--
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-runtime-go] style95 commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
style95 commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937228537
##########
common/gobuild.py.launcher.go:
##########
@@ -89,12 +91,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
if value, ok := input["value"].(map[string]interface{}); ok {
- payload = value
+ payloadForJsonObject = value
+ } else {
+ if value, ok := input["value"].([]interface{}); ok {
+ payloadForJsonArray = value
+ isJsonObjectParam = false
+ }
}
// process the request
- result := action(payload)
+ var result interface{}
+ funcMain := reflect.ValueOf(Main)
Review Comment:
I am not that familiar with the runtime-go code.
Where this `Main` comes from?
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937220952
##########
openwhisk/runHandler.go:
##########
@@ -82,10 +82,14 @@ func (ap *ActionProxy) runHandler(w http.ResponseWriter, r *http.Request) {
// check if the answer is an object map
var objmap map[string]*json.RawMessage
+ var objarray []interface{}
err = json.Unmarshal(response, &objmap)
if err != nil {
- sendError(w, http.StatusBadGateway, "The action did not return a dictionary or array.")
- return
+ err = json.Unmarshal(response, &objarray)
Review Comment:
First, parse the result as json, if failed, parse the result as array.
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937249726
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,28 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
Review Comment:
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.
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936197673
##########
golang1.17/Dockerfile:
##########
@@ -38,7 +38,7 @@ RUN echo "deb http://deb.debian.org/debian buster-backports main contrib non-fre
librdkafka-dev=0.11.6-1.1 &&\
# Cleanup apt data, we do not need them later on.
apt-get clean && rm -rf /var/lib/apt/lists/* &&\
- go get -u github.com/go-delve/delve/cmd/dlv@v1.8.1 &&\
+ go install github.com/go-delve/delve/cmd/dlv@v1.8.1 &&\
Review Comment:
Fix issue
```
go get: installing executables with 'go get' in module mode is deprecated.
Use 'go install pkg@version' instead.
For more information, see https://golang.org/doc/go-get-install-deprecation
or run 'go help get' or 'go help install'.
# github.com/rivo/uniseg
pkg/mod/github.com/rivo/uniseg@v0.3.1/properties.go:130:6: missing function body
pkg/mod/github.com/rivo/uniseg@v0.3.1/properties.go:130:20: syntax error: unexpected [, expecting (
note: module requires Go 1.18
```
##########
build.gradle:
##########
@@ -18,6 +18,7 @@
buildscript {
repositories {
jcenter()
+ mavenCentral()
Review Comment:
Fix
`scalafmt-core` is deprecated in jcenter, should use `mavenCentral()`
And need to keep jcenter() due to exist some lib needs this
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936202270
##########
tests/build.gradle:
##########
@@ -34,9 +34,12 @@ tasks.withType(Test) {
}
dependencies {
- compile "org.scala-lang:scala-library:${gradle.scala.version}"
- compile "org.apache.openwhisk:openwhisk-tests:${gradle.openwhisk.version}:tests"
- compile "org.apache.openwhisk:openwhisk-tests:${gradle.openwhisk.version}:test-sources"
+ implementation "junit:junit:4.11"
Review Comment:
Reason is here: https://github.com/apache/openwhisk-runtime-nodejs/pull/221#discussion_r936174947
--
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-runtime-go] ningyougang commented on pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#issuecomment-1193615557
If want to make travis ci passed, need to merge this pr: openwhisk/pull/5290 firstly
--
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-runtime-go] ningyougang commented on pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#issuecomment-1204588461
@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.
--
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-runtime-go] ningyougang merged pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang merged PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936197673
##########
golang1.17/Dockerfile:
##########
@@ -38,7 +38,7 @@ RUN echo "deb http://deb.debian.org/debian buster-backports main contrib non-fre
librdkafka-dev=0.11.6-1.1 &&\
# Cleanup apt data, we do not need them later on.
apt-get clean && rm -rf /var/lib/apt/lists/* &&\
- go get -u github.com/go-delve/delve/cmd/dlv@v1.8.1 &&\
+ go install github.com/go-delve/delve/cmd/dlv@v1.8.1 &&\
Review Comment:
Fix issue
```
go get: installing executables with 'go get' in module mode is deprecated.
Use 'go install pkg@version' instead.
For more information, see https://golang.org/doc/go-get-install-deprecation
or run 'go help get' or 'go help install'.
# github.com/rivo/uniseg
pkg/mod/github.com/rivo/uniseg@v0.3.1/properties.go:130:6: missing function body
pkg/mod/github.com/rivo/uniseg@v0.3.1/properties.go:130:20: syntax error: unexpected [, expecting (
note: module requires Go 1.18
```
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937221463
##########
golang1.18/lib/launcher.go:
##########
@@ -100,12 +102,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
Review Comment:
Support jsonArray as input param as 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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r928367775
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,27 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ c.init(initPayload(src))._1 shouldBe (200)
+ val result = c.run(JsObject())
+ result._1 shouldBe (200)
Review Comment:
Nomally, the test case here should compare the result as well. e.g.
```
withActionLoopContainer { c =>
c.init(initPayload(src))._1 shouldBe (200)
val result = c.runForJsArray(JsObject())
result._1 shouldBe (200)
result._2.get.elements shouldBe Vector(JsString("a"), JsString("b"))
}
```
But `c.runForJsArray(JsObject())` depend on openwhisk and this method is implelment in this pr: https://github.com/apache/openwhisk/pull/5290/files#r926350523
Due to doesn't know the merge time for openwhisk/pull/5290, so currently, just compare the code here.
If 5290 merged, i will add logic(compare the result) for this test case
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936202270
##########
tests/build.gradle:
##########
@@ -34,9 +34,12 @@ tasks.withType(Test) {
}
dependencies {
- compile "org.scala-lang:scala-library:${gradle.scala.version}"
- compile "org.apache.openwhisk:openwhisk-tests:${gradle.openwhisk.version}:tests"
- compile "org.apache.openwhisk:openwhisk-tests:${gradle.openwhisk.version}:test-sources"
+ implementation "junit:junit:4.11"
Review Comment:
Reason is here: https://github.com/apache/openwhisk-runtime-nodejs/pull/221#discussion_r936174947
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937318974
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj []interface{}) []interface{} {
Review Comment:
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
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937446029
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj []interface{}) []interface{} {
Review Comment:
@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.
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj []interface{}) []interface{} {
Review Comment:
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.
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r928367775
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,27 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ c.init(initPayload(src))._1 shouldBe (200)
+ val result = c.run(JsObject())
+ result._1 shouldBe (200)
Review Comment:
Nomally, the test case here should compare the result as well. e.g.
```
withActionLoopContainer { c =>
c.init(initPayload(src))._1 shouldBe (200)
val result = c.runForJsArray(JsObject())
result._1 shouldBe (200)
result._2.get.elements shouldBe Vector(JsString("a"), JsString("b"))
}
```
But `c.runForJsArray(JsObject())` depend on openwhisk and this method is implelment in this pr: https://github.com/apache/openwhisk/pull/5290/files#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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937284262
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
Review Comment:
Add another test case for support array as 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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937318974
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj []interface{}) []interface{} {
Review Comment:
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.
--
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-runtime-go] style95 commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
style95 commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937277906
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,28 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
Review Comment:
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.
--
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-runtime-go] ningyougang commented on pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#issuecomment-1205897145
I will merge due to `other runtimes depend on this pr`, and because exist 2 test cases，i think it is safe for this pr.
If has issue, open new 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-runtime-go] ningyougang commented on pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#issuecomment-1193615048
The steps of test `Support sequence action`
* Write split.go and sort.go
```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)
```shell
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
```
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r928367775
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,27 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ c.init(initPayload(src))._1 shouldBe (200)
+ val result = c.run(JsObject())
+ result._1 shouldBe (200)
Review Comment:
Nomally, the test case here should compare the result as well. e.g.
```
withActionLoopContainer { c =>
c.init(initPayload(src))._1 shouldBe (200)
val result = c.runForJsArray(JsObject())
result._1 shouldBe (200)
result._2.get.elements shouldBe Vector(JsString("a"), JsString("b"))
}
```
But `c.runForJsArray(JsObject())` depend on openwhisk and this method is implelment in this pr: https://github.com/apache/openwhisk/pull/5290/files#r926350523
Due to doesn't know the merge time for openwhisk/pull/5290, so currently, just compare the code here.
If 5290 merged, i will add logic(compare the result) for this test case
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936202084
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,28 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
Review Comment:
Compare the 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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937253104
##########
common/gobuild.py.launcher.go:
##########
@@ -89,12 +91,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
if value, ok := input["value"].(map[string]interface{}); ok {
- payload = value
+ payloadForJsonObject = value
+ } else {
+ if value, ok := input["value"].([]interface{}); ok {
+ payloadForJsonArray = value
+ isJsonObjectParam = false
+ }
}
// process the request
- result := action(payload)
+ var result interface{}
+ funcMain := reflect.ValueOf(Main)
+ if isJsonObjectParam {
+ param := []reflect.Value{reflect.ValueOf(payloadForJsonObject)}
+ reflectResult := funcMain.Call(param)
Review Comment:
If invokes Main directly, compile would be failed.
So i used `go reflect` to invoke Main
--
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-runtime-go] ningyougang commented on pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#issuecomment-1204674263
@style95 , new test case is added here: https://github.com/apache/openwhisk-runtime-go/pull/170/files#diff-37f21cca37a0702d947ad4948d51808b04eae395f1deb7b3d2d8d51cdef61723R163
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r928367775
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,27 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ c.init(initPayload(src))._1 shouldBe (200)
+ val result = c.run(JsObject())
+ result._1 shouldBe (200)
Review Comment:
Nomally, the test case here should compare the result as well. e.g.
```
withActionLoopContainer { c =>
c.init(initPayload(src))._1 shouldBe (200)
val result = c.runForJsArray(JsObject())
result._1 shouldBe (200)
result._2.get.elements shouldBe Vector(JsString("a"), JsString("b"))
}
```
But `c.runForJsArray(JsObject())` depend on openwhisk and this method is implelment in this pr: https://github.com/apache/openwhisk/pull/5290/files#r926350523
Due to doesn't know the merge time for openwhisk/pull/5290, so currently, just compare the code here.
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937221463
##########
golang1.18/lib/launcher.go:
##########
@@ -100,12 +102,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
Review Comment:
Support jsonArray as input param as well. this is for sequence action
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937221761
##########
golang1.17/lib/launcher.go:
##########
@@ -100,12 +102,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
Review Comment:
Support json array as input param, this is for sequence action
--
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-runtime-go] style95 commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
style95 commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937227690
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,28 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
Review Comment:
So if we want to use the JSON array params, do we have to use this signature for the main function?
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937252815
##########
common/gobuild.py.launcher.go:
##########
@@ -89,12 +91,29 @@ func main() {
}
}
// get payload if not empty
- var payload map[string]interface{}
+ isJsonObjectParam := true
+ var payloadForJsonObject map[string]interface{}
+ var payloadForJsonArray []interface{}
if value, ok := input["value"].(map[string]interface{}); ok {
- payload = value
+ payloadForJsonObject = value
+ } else {
+ if value, ok := input["value"].([]interface{}); ok {
+ payloadForJsonArray = value
+ isJsonObjectParam = false
+ }
}
// process the request
- result := action(payload)
+ var result interface{}
+ funcMain := reflect.ValueOf(Main)
Review Comment:
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.
--
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-runtime-go] upgle commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
upgle commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r937316863
##########
tests/src/test/scala/runtime/actionContainers/ActionLoopGoContainerTests.scala:
##########
@@ -135,4 +135,51 @@ abstract class ActionLoopGoContainerTests
c.run(helloMsg()) should be(okMsg("hello-Hello", "Hello, Demo!"))
}
}
+
+ it should "support return array result" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj map[string]interface{}) []interface{} {
+ | result := []interface{}{"a", "b"}
+ | return result
+ |}
+ |
+ """.stripMargin
+ }
+ val src = ExeBuilder.mkBase64SrcZip(
+ Seq(
+ Seq(s"main.go") -> helloArrayGo
+ ))
+ withActionLoopContainer { c =>
+ 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")))
+ }
+ }
+
+ it should "support array as input param" in {
+ val helloArrayGo = {
+ s"""
+ |package main
+ |
+ |func Main(obj []interface{}) []interface{} {
Review Comment:
I wonder what happens when the argument of the main function is an array and an object parameter is passed.
--
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-runtime-go] ningyougang commented on a diff in pull request #170: Support array result include sequence action
Posted by GitBox <gi...@apache.org>.
ningyougang commented on code in PR #170:
URL: https://github.com/apache/openwhisk-runtime-go/pull/170#discussion_r936197772
##########
build.gradle:
##########
@@ -18,6 +18,7 @@
buildscript {
repositories {
jcenter()
+ mavenCentral()
Review Comment:
Fix
`scalafmt-core` is deprecated in jcenter, should use `mavenCentral()`
And need to keep jcenter() due to exist some lib needs this
--
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