Allow Context to be set in http.Request for Fastly API calls

[alwin-stripe]

Hi there,

I have a use-case where we are using an egress proxy to make API calls to Fastly and would like to set a context.Context in http.Request before making the http request.

My proposal is to update RequestOptions to include an additional Context attribute and create new methods in Client with a WithCtx(...) suffix. See example below.

// RequestOptions is the list of options to pass to the request.
type RequestOptions struct {
	// Body is an io.Reader object that will be streamed or uploaded with the
	// Request.
	Body io.Reader
	// BodyLength is the final size of the Body.
	BodyLength int64
	// Headers is a map of key-value pairs that will be added to the Request.
	Headers map[string]string
	...
        ...
        ...

        // Context is a context.Context object that will be set to the Request's context.
        Context context.Context
}

// CreatePrivateKeyWithCtx creates a new resource using given context
func (c *Client) CreatePrivateKeyWithCtx(ctx context.Context, i *CreatePrivateKeyInput) (*PrivateKey, error) {
	p := "/tls/private_keys"

	if i.Key == "" {
		return nil, ErrMissingKey
	}

	if i.Name == "" {
		return nil, ErrMissingName
	}

	var ro *RequestOptions
	if ctx != nil {
		ro = new(RequestOptions)
		ro.Context = ctx
	}

	r, err := c.PostJSONAPI(p, i, ro)
	if err != nil {
		return nil, err
	}

	var ppk PrivateKey
	if err := jsonapi.UnmarshalPayload(r.Body, &ppk); err != nil {
		return nil, err
	}

	return &ppk, nil
}

// CreatePrivateKey creates a new resource.
func (c *Client) CreatePrivateKey(i *CreatePrivateKeyInput) (*PrivateKey, error) {
	return c.CreatePrivateKeyWithCtx(nil, i)
}

Tagging @Integralist and @joeshaw for opinions/suggestions.

Thanks,
Alwin

[Integralist]

Hi @alwin-stripe

Thanks for opening this issue.

So a couple of things to mention...

1. It makes sense to support contexts (thank you for the suggestion).
2. If we did implement, it's more likely to be a breaking change (i.e. new major release) rather than adding new WithCtx methods.

I've created an internal ticket for this request but I'm unable to say when it would be done (or even if it will be done).

To clarify the latter point, there's one thing that makes adding your requested feature more complicated, and that is we now code generate our API clients (see this blog post explaining how it works). So we now have https://github.com/fastly/fastly-go which is code generated from our (currently internal) OpenAPI specification.

The new fastly-go API client doesn't yet support all the Fastly API endpoints (see this section of its README for details) but it does support contexts. So it might be worth you investigating the use of that newer client to see if it's suitable for your needs.

[alwin-stripe]

Hi @Integralist

Thanks for the suggestion to use the new fastly-go API client. Migrating to the new API client across our mono-repo requires more work on our end.

I managed to work around this by overriding the HTTPClient parameter of fastly.Client with an http client which uses a custom RoundTripper that sets the appropriate context on each request.

Thanks!

[Integralist]

Great. Good to hear you have a workaround for the time being. I appreciate the update 👍🏻