Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pure-JS actions are not wrapped #334

Closed
tripodsan opened this issue Nov 19, 2018 · 10 comments · Fixed by #363
Closed

Pure-JS actions are not wrapped #334

tripodsan opened this issue Nov 19, 2018 · 10 comments · Fixed by #363
Assignees
Labels
bug Something isn't working

Comments

@tripodsan
Copy link
Contributor

Is your feature request related to a problem? Please describe.
as discussed in #328, pure-js actions (eg: html.js or html_dump.js are currently not transformed/wrapped by parcel, and end up as-is in the openwhisk action. this means the developer needs to write (and understand) the openwhisk web action invocation semantics.

Describe the solution you'd like
Ideally, such pure-js actions are automatically wrapped with parcel so that the developer can write them the same way as the ...pre.js actions.

Describe alternatives you've considered
the only alternative is to write a pre.js and have an empty .htl

@tripodsan tripodsan self-assigned this Nov 19, 2018
@trieloff
Copy link
Contributor

@rofe you've been experiencing this.

@trieloff trieloff added the bug Something isn't working label Nov 20, 2018
tripodsan added a commit that referenced this issue Nov 21, 2018
@trieloff
Copy link
Contributor

@tripodsan did the merge of #325 fix this?

@tripodsan
Copy link
Contributor Author

did the merge of #325 fix this?

no. not yet. if you write a pure-js action, it will be turned into a pure openwhisk action.

@tripodsan
Copy link
Contributor Author

with this fixed, pure action can be written with a simple main pipeline function. for example:

src/xml.js

module.exports.main = async function main() {
  return {
    body: `<?xml version="1.0" encoding="UTF-8"?>
<note>
  <to>Peach</to>
  <from>Mario</from>
  <heading>Reminder</heading>
  <body>Don't forget me this weekend!</body>
</note>`,
  };
};

@rofe
Copy link
Contributor

rofe commented Nov 29, 2018

Not sure yet if this is a helix-cli or helix-pipeline (specifically #106?) problem, but my example of a pure js action fails in the validate action due to an interestingly looking response object during step 6 after the main function (where I simply return payload) got executed:

main function in test_xml.js:

async function main(payload, action) {
    return payload;
}

payload dump:

{
    "content": {
        ...
    },
    "request": {
        ...
    },
    "response": {
        "request": {
            ...
	},
	"content": {
	    ...
        }
    }
}

Looks like at some point response got confused with payload and request and content objects got duplicated inside...
@trieloff , @tripodsan

@rofe
Copy link
Contributor

rofe commented Nov 29, 2018

The response seems to be merged into payload right after the 'main' function. The error message is Error: Invalid Context at step 6 data.response should NOT have additional properties, data.response should NOT have additional properties

@rofe rofe reopened this Nov 29, 2018
@trieloff
Copy link
Contributor

Step 6 is once, so it's unlikely to happen in the pipeline, but in the wrapping code.

@trieloff
Copy link
Contributor

trieloff commented Nov 29, 2018

@rofe try changing two things:

  1. in pipeline: content.xml to response.xml (also in the schema)
  2. in your main: return {xml: payload};

@tripodsan
Copy link
Contributor Author

the pure-js action, should act like a pipeline function, so @rofe's example above should work, IMO.
the problem is, that the generated htl script does not comply with the pipeline function, so the wrapper is wrong.

@rofe
Copy link
Contributor

rofe commented Nov 30, 2018

Thanks @tripodsan it works now. One just needs to be careful to add a response object in the main function (at the latest).
@trieloff not sure I follow your proposal... let's maybe continue this change request in a separate issue in helix-pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants