Switch to SDK where possible

[alexellis]

Switch to faas-cli SDK where possible for invoking endpoints.

[viveksyngh]

I can work on this one.

[viveksyngh]

Derek assign: me

[alexellis]

@viveksyngh @LucasRoesler I wonder if we need to consider adding HTTP status codes to the response of API calls in the SDK? i.e. like github-go

[LucasRoesler]

It could help, but I think it is probably most important for the invoke method. For all of the other endpoints the status codes are known and can easily correspond to known errors. It would also be straightforward to wrap the status code into the error object and provide a nice helper function to pull it out. Something like StatusFromError(err)

[alexellis]

The reason why I'm mentioning the status codes is that the certifier originally picked up issues with differing status codes with faas-netes vs faas-swarm, and should ideally agree with what's in the Swagger definition and visa versa.

[LucasRoesler]

I understand the point, perhaps i can be more clear/explicit.

I think there are two options:

1. All proxy methods have the following return signature (resp interface{}, status int, err error) where resp is of course typed per what ever makes sense for that method.
2. We define and always use a new error struct that encodes information like the status code

   
   // ProxyError is an error object for proxy client requests.
   // It can contain the status code, if a response was received
   // or the Go client error if one occurred.
   type ProxyError struct {
   // Status is the status code from the api response
   Status int
   // Err is the go http client error, if any occurred
   // these are the errors you get from `client.Do`
   Err error
   msg string
   }

func (e *ProxyError) Error() string { return e.msg }

func StatusFromError(err error) int { switch e := err.(type) { case *ProxyError: return e.Status default: // or maybe return http.StatusInternalServiceError instead? return 0 } }

This method `StatusFromError` isn't truly needed, especially in go 1.14 we can use the new `errors.As` method

```go
resp, err := proxy.Deploy(....)
var e *ProxyError
if errors.As(err, e) {
    // now do stuff that checks the status code
    if e.Err != nil {
        // handle client errors, probably return or exit early here
    }
    // now check status codes here if need to 
    switch e.Status {
    case http.StatusNotFound:
        //....
    }
}

If we go this route, we would also want to provide some sentinel errors for common status codes, e.g.

ProxyNotFound = ProxyError{404, nil, "endpoint not found"}
ProxyUnauthorized = ProxyError{401, nil, "unauthenticated request"}

and these can be used very nicely with errors.Is like this

if errors.Is(err, ProxyNotFound) { ... }

To me, the primary benefit of option (2) is that most methods know the exact success code and for most callers anything but that code is an error and an exit. This means most uses of the proxy will just drop the status code value, like this resp, _, err := proxy.Method(..). Using option (2) means we can keep the cleaner (and importantly backwards compatible) resp, err := proxy.Method(..) while still having access to the status code when we need it.

IMO, each endpoint should only have a single success status and anything outside of the range should be considered a bug and not certified. The proxy code knows how to determine which code is a success and otherwise return an error for anything else and with option (2) the certifier can check and enforce this.

[viveksyngh]

IMO providing the entire response objects or (response body and status code) along with typed return value and error will be good.

something as below

func (s *client) ListFunctions(ctx context.Context, namespace string) ([]*types.FunctionStatus, *Response, error) {

This gives the caller flexibility to write logic based on different status codes and consume the response in different way. Still, we can provide some sentinel errors for common status codes.