bufbuild / httplb

Client-side load balancing for net/http
https://pkg.go.dev/github.com/bufbuild/httplb
Apache License 2.0
48 stars 2 forks source link

API improvements, to get the repo closer to open-sourcing #51

Closed jhump closed 1 year ago

jhump commented 1 year ago

I've put each change into its own commit, so hopefully this diff (which is rather large) can be digested more easily if reviewing commit-by-commit.

The gist of the changes, after discussing with @akshayjshah:

  1. The name Attrs, while idiomatic in most Go codebases, will not pass muster as an abbreviation. And Attributes is rather long and annoying to type. Solution: move (back) to another package, so it's called attribute.Values instead of resolve.Attrs. If user wants shorter name, they could use an import alias and then use attr.Values in their code. Moving back to its own package also makes names a little more clear (Key, Values, GetValue, instead of AttrKey, Attrs, AttrValue).
  2. Names like "factory" also elicit an allergic reaction. For picker.Factory, it turns out that just removing it and using a plain function type makes things simpler. There is a bit of a downside in that function signatures that accept the function as an argument are a little longer and less readable-at-a-glance. But there was only one such instance in the exported API (httplb.WithPicker).
  3. Following on the previous bullet: we rename RoundTripperFactory to Transport; its New method is renamed NewRoundTripper. Similarly, we rename RoundTripperOptions to TransportConfig. When using the options-struct pattern, there is a preference to name the struct "Config" instead of "Options". (The name "Option" is reserved for use with the functional-options pattern.)
  4. Let's not commit to Client.Prewarm functionality just yet. So this PR unexports it. We can create a discussion in the public repo to figure out what requirements might be for this and expose it later if/when we have a better sense for the best API.
  5. Simplify httplb.Client but not allowing per-target options. If you need per-target options, create multiple clients instead. This allows us to merge ClientOption and TargetOption into one type. And now, when you allow a target, it is only one allowed target and automatically disallows other targets (which allows us to remove the WithDisallowUnconfiguredTargets option).