Updated BrokerMetadata#nodeId type from int to int | str.

[jackgene]

Changes

Fixes #1050

Preferred (less invasive) fix to issue above, changes the type of aiokafka.structs.BrokerMetadata#nodeId from int to int | str. No functionality change.

Use this or #1052, but not both.

Checklist

The check list mentions a CHANGES folder, but I do not see one, am I supposed to create one for each PR?

[ods]

Hi @jackgene, thank you for the contribution. You're right, the the type annotation doesn't match the code. But I'm not sure we need to fix the type annotation here and not the code.

[jackgene]

Hi @jackgene, thank you for the contribution. You're right, the the type annotation doesn't match the code. But I'm not sure we need to fix the type annotation here and not the code.

Yup, and I did offer two PRs, though they both touch the type annotation.

As I understand it, in the aiokafka model, node_id is used for two things:

• int ID, as assigned by Kafka
• Synthetically generated str ID to denote broker/coordinator by the aiokafka library

We obviously do not want the two to collide, and it won't in the current implementation, as one is an int and the other is a str. We can keep things the way they are, and make it explicit that the type of node_id is int | str.

Alternatively, we can make it a single type, str (in my other PR), makes it easy to distinguish between the two classes of ids (numeric strings, vs strings prefixed by broker-/coordinator-).

We could also make it an int, but this require knowledge of how Kafka assigns IDs that I'm unfortunately not familiar with. If Kafka only assigns positive int for instance, we could use negative ints to represent broker/coordinators.

So IMO, changing the type annotation is the less invasive option.

[ods]

Ok, as a temporary solution it's reasonable to keep it as-is. Let's go this way. Please rebase it and add a FIXME comment explaining why it's defined this way