Documentation for AdminClient API about franz-go HOT 6 CLOSED

Hey there! I just replied on your other issue, it sounds like some examples would be beneficial.

In this issue, are you thinking that the code generator readme needs updating / clarifying? (for what it's worth, it is out of date)

Or do you mean that the code in the DEFINITIONS file could use more documentation, so that the kmsg package is a bit simpler to understand?

Or something else, or both of the above in addition to something else?

from franz-go.

I think this issue is not needed anymore as the other issue kinda covers this one.

Regarding the definitions, after looking it up again it's clearer. I wonder where you got all the information from for the DEFINITIONS file. From the Java client? A reference is helpful so that I can look up what API version I should use for my expected Kafka version when using KMsgs directly (as needed for the Admin client).

One more opinion regarding the generated code: It might be personal preference but I think having a separate go file for each struct would make it more comfortable to explore the code.

One last question for today:

I can see that I am in charge of specifying the response type on my own after sending the request. Is it a desirable goal to offer an interface for that which doesn't require type cases, takes care of response validation etc.? So instead of this:

kresp, err := cl.Client().Request(context.Background(), req)
resp := kresp.(*kmsg.DescribeACLsResponse)
if resp.ErrorCode != nil {}
// ....

an interface like this:

func (c *Client) DescribeACLs(ctx context.Context, req *msg.DescribeACLsRequest) (*DescribeACLsResponse, error) {}

As far as I got this way we could make it safe to use latest as max version again right?

Edit: Do you want to create a Discord Server or Slack channel for further discussions? Would love to discuss more in the future :-)

from franz-go.

I crated a discord (also signed up for discord, so if this link doesn't work let me know):
https://discord.com/channels/762731299294412863/762731299790127136

The DEFINITIONS file has been pulled from clients/src/main/resources/common/message in the kafka repo. I initially tried following KIPs, but they're often out of date from what the source actually is.

I've been thinking to split the DEFINITIONS file into one file per request&response. I could theoretically do the same for the generated code, but I didn't think that people would be looking at the source of the autogenerated code, just the documentation. What's the benefit to split out the generated requests and responses?

I've avoided a dedicated API on the client so far to avoid a balooning method list of the client. I figured that having a universal Request method that did the right thing would be preferable, but it definitely does lack type safety. I'm not sure how worth it it is to add the type safety-ness. However, it would be pretty easy to add some type safety to the kmsg package itself with autogenerated code, and since that package is already bloated (through the 51 requests & responses and all of their methods), adding another method would not be too bad.

For example,

type Requestor interface {
  Request func(context.Context, kmsg.Request) (kmsg.Response, error)
}

and then for each type, have a function that looks like

func (v *MetadataRequest) RequestWith(r Requestor) (*MetadataResponse, error)

What do you mean about using latest as the max version? Do you mean to not have the client automatically bump to the latest version if Kafka supports it, if you do not directly set a max version?

from franz-go.

The discord link is wrong I think. You have to invite people to your Discord Server (similar to slack environments): https://i.imgur.com/vFibhF0.png . There you can get an invite link, which you can publish here :).

What's the benefit to split out the generated requests and responses?

People like me who use the kmsg package directly can easier find what they are looking for (I don't necessarily know the package names, but when I see the name I know it's the right one basically. But no big deal, just a preference from my side.

What do you mean about using latest as the max version? Do you mean to not have the client automatically bump to the latest version if Kafka supports it, if you do not directly set a max version?

Yes that's what I meant.

from franz-go.

Thanks for the quick pic, makes sense; here’s a permanent link:

https://discord.gg/4Df7v49

We’ll have to talk about the versioning thing, but I think I know what you mean, and I’ll have to think about some api aspects of it—it may necessitate a new VersionedRequest function.

from franz-go.

I've split the DEFINITIONS file into one file per request, but I don't think it's worth it to split up the generated Go code. That code shouldn't really be looked at, but it is easy enough in godoc or pkg.go.dev to click through and view the source of a function / type you're interested in.

I've added the Requestor interface, and a RequestWith to every Request type. The RequestWith is autogenerated, but is useful and provides the type safety niceness (somewhat, since the function just does the type asserting internally, but it is convenient).

I think all that remains for this ticket is "A reference is helpful so that I can look up what API version I should use for my expected Kafka version when using KMsgs directly (as needed for the Admin client).", but this has mostly been solved with the new Default / New. These functions should make requests forward compatible: so long as you Default, if you bump versions and do not specify new fields, you should get safe defaults

As far as determining which API version to use based off your Kafka, Kafka actually doesn't make this obvious nor document which Kafka version introduced which request versions. I've manually kept that up to date with my kversion package. Maybe it would be worth it to document that package more? The kversion source itself references the KIP, commit, and KAFKA issue that introduced everything. Nothing like that exists in the Kafka repo.

I think for the most part this is closed, but feel free to reopen this or open a new ticket if I missed anything (I have some sense that I missed out on some documentation, but maybe not?)

from franz-go.