search

openfaas-incubator / node8-express-template

Node.js 8 template for OpenFaaS with HTTP via Express.js
MIT License
15 stars 18 forks source link

Add support for DELETE, PATCH, and PUT methods #17

Closed kturcios closed 5 years ago

kturcios commented 5 years ago

This PR is for adding the support for DELETE, PATCH, and PUT method types.

Resolves openfaas/store#61 Resolves openfaas/faas#815

Signed-off-by: Kevin Turcios kevin_turcios@outlook.com

alexellis commented 5 years ago

Hi Kevin,

Thanks for raising a PR. Please could you see the contribution guide which asks contributors to raise issues to propose changes?

One of the questions that the contribution guide asks you to write about is how changes have been tested and what output can be attached to show that.

If merged it'll need to be done on the node10-* repo too.

Thanks,

Alex

kturcios commented 5 years ago

I see that this following issue was closed because it says that the template supports DELETE and PUT, but I don't believe that is the case?

Current Behaviour

Performing a DELETE request on the following handler:

module.exports = (event, context) => {
    context.succeed("Method: " + event.method);
}

Results in 

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <title>Error</title>
    </head>
    <body>
        <pre>Cannot DELETE /</pre>
    </body>
</html>

I was a little confused as to what to do since the issue was closed. Do you still want me to raise a new issue for this again?

ewilde commented 5 years ago

This relates to https://github.com/openfaas/faas/issues/815 support for additional http verbs. @kturcios do you want to add PATCH verb as well?

alexellis commented 5 years ago

@kturcios please raise an issue on the openfaas/store repo to track this.

Let's make sure that all templates which we track officially support:

            http.MethodPost
            http.MethodPut
            http.MethodPatch
            http.MethodDelete
            http.MethodGet

This only applies to templates based upon of-watchdog.

Alex

kturcios commented 5 years ago

Closing in place of the updated node10-express template.