feat(sdk/go): Refactor the key/value Go SDK to be more idiomatic

[adamreese]

• Introduces a client struct responsible for handling the connection
• Remove exposed C types

[vdice]

A reminder to track or line up updates to examples/docs in https://github.com/fermyon/developer on/around the 2.0 release.

[rylev]

I have a concern with releasing this with 2.0. The Go SDK will continue to target the old Spin 1.0 world with the release of Spin 2.0. However, we will soon want the Go SDK to target the 2.0 world. This will require additional breaking changes to the Spin SDK to make work. This leads to a versioning challenge - if we release Go SDK 2.0 with these changes, we will then want to release Go SDK 3.0 with the changes need to target Spin 2.0's new world. This leads to the SDK and Spin's version drifting.

[lann]

Spin 2.0's new world

Can we make the Go interface compatible with the new world, "adapting" to the old world?

Update: Taking a closer look, I think this is entirely plausible with the implementation in this PR. I would say that translating the new world's use of none to indicate "no such key" into a ErrNoSuchKey would be ideomatic for Go anyway.

[lann]

These should be exported Err* constants so that callers can distinguish between error types.

[adamreese]

I was going to include that in a separate PR to make all the SDK's consistent. I wasn't sure yet if a custom error type would be the way to go.

[lann]

Sure, that's fine. As a general approach I would probably do something like:

// Named const for payload-less variants
var ErrAccessDenied = errors.New("access denied")

// Error type for most payload-ful variants (not KV)
type ImaginaryError string

// Anonymous errors for `other(string)` variants
errors.New(otherString)

[adamreese]

I was thinking about something like...

type Error struct {
  Code int
  Message string
}

Stdlib example https://github.com/golang/go/blob/master/src/net/textproto/textproto.go#L35

But it's still a little awkward because Code is an internal value that has no real meaning to the user

[lann]

It depends on whether we care more about trying to closely match the WIT or trying to write a "idiomatic" Go lib I think. For the latter I would say separate var Err*s for no-payloads and type *Errors for yes-payloads most closely matches the ecosystem.

[lann]

Taking protobuf as an example:

no-payload:
https://pkg.go.dev/github.com/golang/protobuf/proto#pkg-variables
yes-payload:
https://pkg.go.dev/github.com/golang/protobuf/proto#ParseError
https://pkg.go.dev/github.com/golang/protobuf/proto#RequiredNotSetError

[adamreese]

I'm sold. I was getting distracted by the wit. Thanks for the journey.

[adamreese]

@rylev What changes to the public API were you thinking would need to be changed for migrating to the 2.0 world? My understanding of worlds and adapters shenanigans is limited.

[lann]

@rylev What changes to the public API were you thinking would need to be changed for migrating to the 2.0 world? My understanding of worlds and adapters shenanigans is limited.

The biggest difference is a switch to resources, which shouldn't make much of a difference to the exported Go interface (just the implementation, once resources are supported).

There are however a bunch of other small changes to the 2.0 interfaces. For KV specifically the biggest small change is the return type of get switching from result<list<u8>, error> to result<option<list<u8>>, error>, where it will return ok(none) if a key isn't found rather than err(no-such-key). For the Go interface I think it will make sense to translate that ok(none) back into a e.g. ErrNoSuchKey error anyway.

[endocrimes]

Do we want to start only building these when building for WASI and otherwise provide a no-op stub? It'll make building mixed spin/non-spin codebases easier.

[lann]

Seems fine in theory. Would you copy all the function stubs or is there something to automate that?