Introduce context method parameters

[displague]

The packngo library does not currently allow for context deadlines to be used in requests.

https://blog.golang.org/context-and-structs

Context makes it easy to propagate important cross-library and cross-API information down a calling stack. But, it must be used consistently and clearly in order to remain comprehensible, easy to debug, and effective.
When passed as the first argument in a method rather than stored in a struct type, users can take full advantage of its extensibility in order to build a powerful tree of cancelation, deadline, and metadata information through the call stack. And, best of all, its scope is clearly understood when it's passed in as an argument, leading to clear comprehension and debuggability up and down the stack.
When designing an API with context, remember the advice: pass context.Context in as an argument; don't store it in structs.

More at https://blog.golang.org/context

What would it look like to add Context to packngo at this time?

• What level of version change will this introduce?
• Will the existing API be maintained, and new methods added?
• Do we create packages for each service, introducing contexts, and making the root package wrap these?
• Do we wait for Take a serious look at generating this client from Swagger #215 or Equinix Metal Go bindings #213?

[tobert]

Oh hey, I was looking at this today.

One idea I had was we could add new methods that accept context and lace it through the code, and provide the current API via wrappers that simply add context.Background().

e.g.

func DoThingCtx(ctx context.Context, arg1 string) error {}
func DoThing(arg1 string) error {
    return DoThingCtx(context.Background(), arg1)
}

In my case, this would allow for OpenTelemetry trace & span propagation through the library for everyone that uses context.

[t0mk]

@displague @tobert Hi, I'm adding my 2 cents.

I'm pro introducing context methods. I am against waiting for swagger generated client. Pairing DoThingCtx and DoThing (like @tobert wrote) sounds good. I wouldn't change the structure of the service, just adding the Ctx methods seems good enough to me.

• ad Swagger generated client
  • we have experience that the API json doesn't always reflect current state of the API, tweaks are necessary
  • no other infra providers really use a generated client, there might be a reason that we don't see and might not be worth the effort to investigate

We might need the context functions for terraform provider at some point, the basic CRUD functions are mentioned as "deprecated" in the TK SDK:
https://github.com/hashicorp/terraform-plugin-sdk/blob/main/helper/schema/resource.go#L100

I can start working on this if you want. I'd first create a context client, pick one simple service (maybe projects), and adapted to context methods as @tobert wrote. We could then iterate how it looks, and see how to proceed.

[tobert]

@t0mk this client isn't really my area of focus but if you want to take a swing I'd be happy to help review & champion. A bunch of internal Metal uses of packngo would benefit from context passing as well.

[displague]

What level of version change will this introduce?

Playing devil's advocate for a moment, we are still in 0. version land. We could just introduce the leading context parameter.

If we do, I'm sure third-party integrators will step into this on their next go mod tidy.