X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/5ce55817039488a4445287191025212a14e50de8..e065d5863b9b36c1cd221f676baffa57e20e7498:/sdk/go/arvados/client.go diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go index 58c0de8255..5ec828667f 100644 --- a/sdk/go/arvados/client.go +++ b/sdk/go/arvados/client.go @@ -9,6 +9,7 @@ import ( "context" "crypto/tls" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -20,7 +21,7 @@ import ( "strings" "time" - "git.curoverse.com/arvados.git/sdk/go/httpserver" + "git.arvados.org/arvados.git/sdk/go/httpserver" ) // A Client is an HTTP client with an API endpoint and a set of @@ -57,22 +58,31 @@ type Client struct { // HTTP headers to add/override in outgoing requests. SendHeader http.Header + // Timeout for requests. NewClientFromConfig and + // NewClientFromEnv return a Client with a default 5 minute + // timeout. To disable this timeout and rely on each + // http.Request's context deadline instead, set Timeout to + // zero. + Timeout time.Duration + dd *DiscoveryDocument - ctx context.Context + defaultRequestID string + + // APIHost and AuthToken were loaded from ARVADOS_* env vars + // (used to customize "no host/token" error messages) + loadedFromEnv bool } -// The default http.Client used by a Client with Insecure==true and -// Client==nil. +// InsecureHTTPClient is the default http.Client used by a Client with +// Insecure==true and Client==nil. var InsecureHTTPClient = &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true}}, - Timeout: 5 * time.Minute} + InsecureSkipVerify: true}}} -// The default http.Client used by a Client otherwise. -var DefaultSecureClient = &http.Client{ - Timeout: 5 * time.Minute} +// DefaultSecureClient is the default http.Client used by a Client otherwise. +var DefaultSecureClient = &http.Client{} // NewClientFromConfig creates a new Client that uses the endpoints in // the given cluster. @@ -87,6 +97,7 @@ func NewClientFromConfig(cluster *Cluster) (*Client, error) { Scheme: ctrlURL.Scheme, APIHost: ctrlURL.Host, Insecure: cluster.TLS.Insecure, + Timeout: 5 * time.Minute, }, nil } @@ -116,6 +127,8 @@ func NewClientFromEnv() *Client { AuthToken: os.Getenv("ARVADOS_API_TOKEN"), Insecure: insecure, KeepServiceURIs: svcs, + Timeout: 5 * time.Minute, + loadedFromEnv: true, } } @@ -131,11 +144,12 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { } if req.Header.Get("X-Request-Id") == "" { - reqid, _ := req.Context().Value(contextKeyRequestID{}).(string) - if reqid == "" { - reqid, _ = c.context().Value(contextKeyRequestID{}).(string) - } - if reqid == "" { + var reqid string + if ctxreqid, _ := req.Context().Value(contextKeyRequestID{}).(string); ctxreqid != "" { + reqid = ctxreqid + } else if c.defaultRequestID != "" { + reqid = c.defaultRequestID + } else { reqid = reqIDGen.Next() } if req.Header == nil { @@ -144,7 +158,36 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { req.Header.Set("X-Request-Id", reqid) } } - return c.httpClient().Do(req) + var cancel context.CancelFunc + if c.Timeout > 0 { + ctx := req.Context() + ctx, cancel = context.WithDeadline(ctx, time.Now().Add(c.Timeout)) + req = req.WithContext(ctx) + } + resp, err := c.httpClient().Do(req) + if err == nil && cancel != nil { + // We need to call cancel() eventually, but we can't + // use "defer cancel()" because the context has to + // stay alive until the caller has finished reading + // the response body. + resp.Body = cancelOnClose{ReadCloser: resp.Body, cancel: cancel} + } else if cancel != nil { + cancel() + } + return resp, err +} + +// cancelOnClose calls a provided CancelFunc when its wrapped +// ReadCloser's Close() method is called. +type cancelOnClose struct { + io.ReadCloser + cancel context.CancelFunc +} + +func (coc cancelOnClose) Close() error { + err := coc.ReadCloser.Close() + coc.cancel() + return err } func isRedirectStatus(code int) bool { @@ -174,6 +217,8 @@ func (c *Client) DoAndDecode(dst interface{}, req *http.Request) error { return err } switch { + case resp.StatusCode == http.StatusNoContent: + return nil case resp.StatusCode == http.StatusOK && dst == nil: return nil case resp.StatusCode == http.StatusOK: @@ -186,7 +231,7 @@ func (c *Client) DoAndDecode(dst interface{}, req *http.Request) error { return nil case isRedirectStatus(resp.StatusCode): // Copy the redirect target URL to dst.RedirectLocation. - buf, err := json.Marshal(map[string]string{"RedirectLocation": resp.Header.Get("Location")}) + buf, err := json.Marshal(map[string]string{"redirect_location": resp.Header.Get("Location")}) if err != nil { return err } @@ -266,14 +311,21 @@ func anythingToValues(params interface{}) (url.Values, error) { // // path must not contain a query string. func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.Reader, params interface{}) error { - return c.RequestAndDecodeContext(c.context(), dst, method, path, body, params) + return c.RequestAndDecodeContext(context.Background(), dst, method, path, body, params) } +// RequestAndDecodeContext does the same as RequestAndDecode, but with a context func (c *Client) RequestAndDecodeContext(ctx context.Context, dst interface{}, method, path string, body io.Reader, params interface{}) error { if body, ok := body.(io.Closer); ok { // Ensure body is closed even if we error out early defer body.Close() } + if c.APIHost == "" { + if c.loadedFromEnv { + return errors.New("ARVADOS_API_HOST and/or ARVADOS_API_TOKEN environment variables are not set") + } + return errors.New("arvados.Client cannot perform request: APIHost is not set") + } urlString := c.apiURL(path) urlValues, err := anythingToValues(params) if err != nil { @@ -332,17 +384,10 @@ func (c *Client) UpdateBody(rsc resource) io.Reader { // header. func (c *Client) WithRequestID(reqid string) *Client { cc := *c - cc.ctx = ContextWithRequestID(cc.context(), reqid) + cc.defaultRequestID = reqid return &cc } -func (c *Client) context() context.Context { - if c.ctx == nil { - return context.Background() - } - return c.ctx -} - func (c *Client) httpClient() *http.Client { switch { case c.Client != nil: