Use an interface instead of the struct directly.

[jay-rah-oob]

This can help with testing in implementing packages.

Exposing the Client interface instead of a struct, so that we can test in downstream packages a little easier. (even though the interface is huge :D !! )

I split up the buildPaths and generatePaths functions return arrays of the methodTemplate that they created so that the return could be used within a generateInterfaces function to create the Client interface.

The struct was named from Client to client so that it wasn't returned.

[sudomateo]

This can help with testing in implementing packages.

Exposing the Client interface instead of a struct, so that we can test in downstream packages a little easier. (even though the interface is huge :D !! )

Would you be able to expand on how this large interface helps with testing other packages? I have the following thoughts on it after reading the code.

1. Generally, interfaces are defined in the package that consumes the interface rather than the package that produces the interface. I can understand the desire here to define an interface in this SDK since it's generated and would stay up to date. However, that would seemingly put an unnecessary burden on code that relies on this interface since any update would break code that uses the interface (e.g., adding a new method that's not implemented in the concrete type used for testing).
2. I imagine that most of the methods within this interface would remain unimplemented in the concrete type that's used for testing. Specifically tests would rely on a small subset of functionality from the interface which could easily come from an interface that testing code defines and implements. You see this already with large cloud provider SDKs where testing code defines a slim interface for testing and implements it with a concrete type just for those tests. This would be more resilient than a large interface since the small interface defined in test code would only break when a method that's actually in use changes, not every update like it would with this large interface.

I don't say either of these points to discourage this change. I'm just trying to better understand the use case here so that we don't implement something that will be a headache in practice.

I split up the buildPaths and generatePaths functions return arrays of the methodTemplate that they created so that the return could be used within a generateInterfaces function to create the Client interface.

The struct was named from Client to client so that it wasn't returned.

The code looks fine to me. I've seen other SDKs keep their client type a struct and place fields within that are interface types. That way an SDK could have multiple interfaces for the various different APIs (e.g., an Instances interface, a FloatingIPs interface). This is kinda in the middle of the two points lists above but perhaps something worth looking into.

[jay-rah-oob]

I’ve been going about this wrong, being new-ish to Go. Reading more about it, it clicks and makes more sense now.
https://go.dev/wiki/CodeReviewComments#interfaces

The code looks fine to me. I've seen other SDKs keep their client type a struct and place fields within that are interface types. That way an SDK could have multiple interfaces for the various different APIs (e.g., an Instances interface, a FloatingIPs interface). This is kinda in the middle of the two points lists above but perhaps something worth looking into.

I was already thinking about the splitting up of the client into something like this. Maybe I’ll take a stab at that later.

This whole exercise definitely helped my understanding of the oxide.go code base though. 😀

I’ll be implementing more testing in https://github.com/oxidecomputer/rancher-machine-driver-oxide with this new found knowledge.

[karencfv]

Thanks for taking the time to think about how we can improve our SDK @one-horned-flying!

I was already thinking about the splitting up of the client into something like this. Maybe I’ll take a stab at that later.

For big structural changes to the SDK, we'd really appreciate having a discussion through an issue before an implementation. That way we can go through all of the nuances together, come up with the best solution, and have a clear path forward for when the change is implemented.

We'd love to hear about any ideas, proposals, paper cuts etc!

[jay-rah-oob]

And for the record I was looking into using a mocking library that generates from interfacee, possibly one of these:
https://github.com/ovechkin-dm/mockio
https://github.com/uber-go/mock
But reading more about the Go-way, I'll just make those only needed interfaces in the implementing project.