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 2020/01/23 12:29:57 UTC
[GitHub] [openwhisk-runtime-go] sciabarra opened a new pull request #119:
Ack and versioning [work in progress]
sciabarra opened a new pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119
- introduces the optional "wait for ack" to improve
- adds a version check for actions to ensure they work with the right environment
- logs the errors in stderr instead of returning as a request
The changes are expected to be backward compatible, however the wait for ack option and the version check should not be enabled for runtimes where precompiled actions may exists (in practice, golang 1.11 and golang 1.12).
Still missing unit tests, will be added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370103109
##########
File path: openwhisk/executor.go
##########
@@ -108,15 +115,53 @@ func (proc *Executor) Start() error {
return fmt.Errorf("command exited")
}
Debug("pid: %d", proc.cmd.Process.Pid)
+
go func() {
proc.cmd.Wait()
proc.exited <- true
}()
+
+ // not waiting for an ack, so use a timeout
+ if !waitForAck {
+ select {
+ case <-proc.exited:
+ return fmt.Errorf("command exited")
+ case <-time.After(DefaultTimeoutStart):
+ return nil
+ }
+ return nil
+ }
+
+ // wait for acknowledgement
+ Debug("waiting for an ack")
+ ack := make(chan error)
+ go func() {
+ out, err := proc.output.ReadBytes('\n')
+ if err != nil {
+ ack <- err
+ return
+ }
+ // parse ack
+ var ackData ActionAck
+ err = json.Unmarshal(out, &ackData)
+ if err != nil {
+ ack <- err
+ return
+ }
+ // check ack
+ if !ackData.Ok {
+ ack <- fmt.Errorf("the action did not return a proper acknowledgement")
Review comment:
as a future todo please lift all the error messages that will be user visible to a single place so we can edit and polish them for consistency.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370101693
##########
File path: docs/ACTION.md
##########
@@ -73,10 +73,12 @@ The `actionloop` runtime can execute generic Linux executable in an efficient w
<a name="actionloop">
-### The Action Loop Protocol
+### The Action Loop Protocol
The protocol can be specified informally as follow.
+- Send an acknowledgement if required. If the environment variable `__OW_WAIT_FOR_ACK` is not empty, write on file descriptor 3 the string `{ "ok": true }`.
Review comment:
note in the change log you didn't use the double underscore.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370102390
##########
File path: openwhisk/actionProxy.go
##########
@@ -72,6 +72,18 @@ func NewActionProxy(baseDir string, compiler string, outFile *os.File, errFile *
//SetEnv sets the environment
func (ap *ActionProxy) SetEnv(env map[string]interface{}) {
+ // Propagate proxy version
+ ap.env["__OW_PROXY_VERSION"] = Version
+ // propagate OW_EXECUTION_ENV as __OW_EXECUTION_ENV
+ ee := os.Getenv("OW_EXECUTION_ENV")
Review comment:
why are there OW and __OW for these vars?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] sciabarracom commented on a change in pull
request #119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
sciabarracom commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370159766
##########
File path: openwhisk/actionProxy.go
##########
@@ -72,6 +72,18 @@ func NewActionProxy(baseDir string, compiler string, outFile *os.File, errFile *
//SetEnv sets the environment
func (ap *ActionProxy) SetEnv(env map[string]interface{}) {
+ // Propagate proxy version
+ ap.env["__OW_PROXY_VERSION"] = Version
+ // propagate OW_EXECUTION_ENV as __OW_EXECUTION_ENV
+ ee := os.Getenv("OW_EXECUTION_ENV")
Review comment:
because the actions always uses the __OW variables while the proxy uses the OW variables. In short OW_VARIABLES are parameters for the proxy while the __OW are parameters for the action. It is all documented in the ENVVARS document I added
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] sciabarracom commented on a change in pull
request #119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
sciabarracom commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370158860
##########
File path: openwhisk/actionProxy.go
##########
@@ -72,6 +72,18 @@ func NewActionProxy(baseDir string, compiler string, outFile *os.File, errFile *
//SetEnv sets the environment
func (ap *ActionProxy) SetEnv(env map[string]interface{}) {
+ // Propagate proxy version
+ ap.env["__OW_PROXY_VERSION"] = Version
+ // propagate OW_EXECUTION_ENV as __OW_EXECUTION_ENV
+ ee := os.Getenv("OW_EXECUTION_ENV")
+ if ee != "" {
+ ap.env["__OW_EXECUTION_ENV"] = ee
+ }
+ // require an ack
+ wa := os.Getenv("OW_WAIT_FOR_ACK")
+ if wa != "" {
+ ap.env["__OW_WAIT_FOR_ACK"] = wa
Review comment:
yes ... I mark I want to wait_for_ack in the Dockerfile, but I pass the information to the action with an environment variable with a different name (adding the __)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370102036
##########
File path: docs/ACTION.md
##########
@@ -73,10 +73,12 @@ The `actionloop` runtime can execute generic Linux executable in an efficient w
<a name="actionloop">
-### The Action Loop Protocol
+### The Action Loop Protocol
The protocol can be specified informally as follow.
+- Send an acknowledgement if required. If the environment variable `__OW_WAIT_FOR_ACK` is not empty, write on file descriptor 3 the string `{ "ok": true }`.
Review comment:
or below in the envvars readme.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] sciabarra closed pull request #119: Ack and
versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
sciabarra closed pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370102232
##########
File path: openwhisk/actionProxy.go
##########
@@ -72,6 +72,18 @@ func NewActionProxy(baseDir string, compiler string, outFile *os.File, errFile *
//SetEnv sets the environment
func (ap *ActionProxy) SetEnv(env map[string]interface{}) {
+ // Propagate proxy version
+ ap.env["__OW_PROXY_VERSION"] = Version
+ // propagate OW_EXECUTION_ENV as __OW_EXECUTION_ENV
+ ee := os.Getenv("OW_EXECUTION_ENV")
+ if ee != "" {
+ ap.env["__OW_EXECUTION_ENV"] = ee
+ }
+ // require an ack
+ wa := os.Getenv("OW_WAIT_FOR_ACK")
+ if wa != "" {
+ ap.env["__OW_WAIT_FOR_ACK"] = wa
Review comment:
does this need to be re-exported? it's internal.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370101832
##########
File path: docs/ACTION.md
##########
@@ -108,6 +110,10 @@ In the current actionloop image there is `bash` and the `jq` command, so you can
```bash
#!/bin/bash
+# send an ack if required
+if test -n "$__OW_WAIT_FOR_ACK"
+then echo '{"ok":true}' >&3
Review comment:
```suggestion
then echo '{"ok":true}' >&3
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370101418
##########
File path: CHANGES.md
##########
@@ -16,6 +16,11 @@
# limitations under the License.
#
-->
+# 1.16.0
Review comment:
```suggestion
# 1.16.0 (not yet an Apache release)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] sciabarracom commented on a change in pull
request #119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
sciabarracom commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370157592
##########
File path: docs/ACTION.md
##########
@@ -73,10 +73,12 @@ The `actionloop` runtime can execute generic Linux executable in an efficient w
<a name="actionloop">
-### The Action Loop Protocol
+### The Action Loop Protocol
The protocol can be specified informally as follow.
+- Send an acknowledgement if required. If the environment variable `__OW_WAIT_FOR_ACK` is not empty, write on file descriptor 3 the string `{ "ok": true }`.
Review comment:
I used the double underscore in the actions but the parameters are without, for consistency.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk-runtime-go] rabbah commented on a change in pull request
#119: Ack and versioning [work in progress]
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #119: Ack and versioning [work in progress]
URL: https://github.com/apache/openwhisk-runtime-go/pull/119#discussion_r370103364
##########
File path: openwhisk/executor.go
##########
@@ -108,15 +115,53 @@ func (proc *Executor) Start() error {
return fmt.Errorf("command exited")
}
Debug("pid: %d", proc.cmd.Process.Pid)
+
go func() {
proc.cmd.Wait()
proc.exited <- true
}()
+
+ // not waiting for an ack, so use a timeout
+ if !waitForAck {
+ select {
+ case <-proc.exited:
+ return fmt.Errorf("command exited")
+ case <-time.After(DefaultTimeoutStart):
+ return nil
+ }
+ return nil
+ }
+
+ // wait for acknowledgement
+ Debug("waiting for an ack")
+ ack := make(chan error)
+ go func() {
+ out, err := proc.output.ReadBytes('\n')
+ if err != nil {
+ ack <- err
+ return
+ }
+ // parse ack
+ var ackData ActionAck
+ err = json.Unmarshal(out, &ackData)
+ if err != nil {
+ ack <- err
+ return
+ }
+ // check ack
+ if !ackData.Ok {
+ ack <- fmt.Errorf("the action did not return a proper acknowledgement")
Review comment:
```suggestion
ack <- fmt.Errorf("The action did not initialize properly.")
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services