apache / openwhisk

Apache OpenWhisk is an open source serverless cloud platform
https://openwhisk.apache.org/
Apache License 2.0
6.48k stars 1.16k forks source link

`whisk.done` not available in zip-actions #1439

Closed cbickel closed 7 years ago

cbickel commented 7 years ago

Currently it is not possible to use whisk.done or whisk.error in actions which has been uploaded as zip files. @psuter Promises are working.

Environment details:

{ "name": "my-action", "version": "1.0.0", "main": "index.js", "dependencies" : { } }
  1. Create index.js:
function myAction(args) {
    setTimeout(function() {
        whisk.done({ message: 'Hello World' });
    }, 2000);
    return whisk.async();
}

exports.main = myAction;
  1. Do npm install
  2. zip -r action.zip * Create a zip out of these files.
  3. wsk action create asyncZip --kind nodejs:6 action.zip create an action
  4. Invoke this action

    Provide the expected results and outputs:

I expect that there's no problem on executing the action.

Provide the actual results and outputs:

{
  "error": "An error has occurred: ReferenceError: whisk is not defined"
}
psuter commented 7 years ago

Thanks for the report. While the bug was unintended, I'm tempted to file as "won't fix", with the following rationale:

I propose to make the points above part of the official documentation and proceed with marking the whisk object as unavailable in packaged actions.

Any thoughts?

CC @rabbah @sjfink

Addendum: it turns out AUTH_KEY is not actually available in the environment. I suspect this is an oversight; there is code in the proxy to set it at init time, which is wrong, as the value is communicated at run time (I suspect it wasn't always the case).

cbickel commented 7 years ago

For me, this solution would be fine as well, but I think the best would be, if the api path (e.g. https://openwhisk.ng.bluemix.net/api/v1/) is directly available and not only the IP of the Edge-machine. Will this module be added to the default module, that can be used without the zip-upload? Then we would have the same mechanisms for plain js-actions and zip-js-actions.

sjfink commented 7 years ago

I agree that we should move to the npm solution:

https://github.com/openwhisk/openwhisk/issues/616

rabbah commented 7 years ago

The npm package should resolve the URL for you from the edge server. I expect that we will add the package to the base image but until then you can use the zip approach.

rabbah commented 7 years ago

@psuter you're right (and my fault for misleading you) - the testEnv tests which exists for the python, swift and docker action tests is missing from the nodjs tests. This is because the node actions may run out of the stemcell pool, where the auth value has to be provided at runtime.

rabbah commented 7 years ago

The npm package should resolve the URL for you from the edge server. I expect that we will add the package to the base image but until then you can use the zip approach.

jthomas commented 7 years ago

I could modify the module to pick up credentials from the environment parameters if those values aren't passed in through the constructor. It should still be possible to manually set these values if you want to call resources outside the current activation context.

We started work on getting the module integration tested through the CI here. https://github.com/openwhisk/openwhisk-client-js/issues/3

I've set up a basic Travis CI service for unit testing but need some help with how to configure the integration test system to make this work. We have integration tests defined but need a test platform to run again.

jberstler commented 7 years ago

What is the timeline for implementing the NPM solution? As it currently stands, anyone with existing Node actions that want to make use of the whisk object and the zip feature are faced with the task of rewriting their code.

While I think that having an NPM module is a good long term solution, it seems like we should fix this now, even temporarily, to enable people to use the zip feature.

psuter commented 7 years ago

NPM module is available to all NodeJS containers as of #1539, and is auto-configured through environment variables.

The whisk object has been marked as deprecated.