Proposal to make http response structs public

[jacksontj]

Reference upstream issue: prometheus/prometheus#3615

TLDR; these http structs are currently private, people use them in a lot of packages -- it would be great if we exposed them so others who want to do their own projects don't have to copy/paste the structs from the prometheus/prometheus project.

Thoughts?

[brian-brazil]

Per that issue, these are internals and I don't think we should encourage users to depend on them and then break when we make changes.

[jacksontj]

These structs in the client aren't really internals, they are the struct representation of the API, which has some compatibility guarantees. As it stands now everyone (including this client) copy/pastes the structs, since this is a client library I don't see why we can't expose them.

…

On Wed, May 8, 2019, 1:28 AM Brian Brazil ***@***.***> wrote: Per that issue, these are internals and I don't think we should encourage users to depend on them and then break when we make changes. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#571 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMLPHOMDW2UYA6I2VZ5JSLPUKFLJANCNFSM4HLOC6GQ> .

[brian-brazil]

The API has some guarantees, those structs are internals and do not.

[jacksontj]

What are the guarantees on the API? From my understanding its the (1) response codes and (2) response bodies. These structs are just the go representation of the response body. I can understand that you wouldn't want to have clients importing the structs used in the server directly, but I don't see a reason that you wouldn't want to expose them from the go client (since it is doing the same thing).

[brian-brazil]

If client_golang wants to make them public that's a different thing, however I'd hope we'd isolate users of this package from those details.

[jacksontj]

If client_golang wants to make them public that's a different thing

This issue is only about client_golang; @krasi-georgiev thoughts?

[krasi-georgiev]

Would you mind giving some code example to illustrate the problem and how it will be solved by exposing these structs?

[jacksontj]

Sure, the common issue is that a variety of other services need these structs so we all end up copy/pasting. A few examples:

1. trickster: https://github.com/Comcast/trickster/blob/master/model.go#L24
2. promxy: https://github.com/jacksontj/promxy/blob/master/promhttputil/struct.go
3. thanos: https://github.com/improbable-eng/thanos/blob/eddaf1138a107ed5304d7cfb66a43de31a8e56da/pkg/promclient/promclient.go#L199

[krasi-georgiev]

Thanks.
And can you show an example of how would it look like if these were exported?

I am quite influenced by the talk from William Kenedy which advocates that importing a package should be strictly for its functionality and never for its types.

[jacksontj]

If the types were exported these other systems wouldn't need to redefine the structs. So instead of having local struct definitions (which all have to be copied etc.) they could simply import them.

I am quite influenced by the talk from William Kenedy which advocates that importing a package should be strictly for its functionality and never for its types.

In general I'd agree with this, but in this particular case the functionality we are looking for is handling the responses from the HTTP endpoint -- which in go is done by tags in structs. Especially when you look at things like Status and ErrorType those are very similar to stdlib's HTTP status, method, etc. types that it exports.

[krasi-georgiev]

I also see the big benefit for this, but the fact that at the end the users will still import the package just for these exported structs and not for its functionality will cause an additional dependency and limit portability. It is the same as the problem described in this part of the talk which has proven to be a bad idea long term.
I think the Prometheus common package suffers from the same problem.

The benefits of avoiding duplicating types in different packages are great, but isolation and portability is a better argument to keep these unexported I think.

Of course I might be wrong so happy to read more opinions.

[beorn7]

With six weeks of no opinions added here, I'm closing this, as both @krasi-georgiev and @brian-brazil are seeing the trade-off as being better to not export the structs.

Whoever wants to add something new to the debate, please follow up.
To get input from others that haven't seen this issue yet, the prometheus-developers mailing list might be a better forum, though.