Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MINOR] structs.BrokerMetadata#nodeId incorrectly typed #1050

Closed
jackgene opened this issue Oct 7, 2024 · 0 comments · Fixed by #1051 · May be fixed by #1052
Closed

[MINOR] structs.BrokerMetadata#nodeId incorrectly typed #1050

jackgene opened this issue Oct 7, 2024 · 0 comments · Fixed by #1051 · May be fixed by #1052

Comments

@jackgene
Copy link
Contributor

jackgene commented Oct 7, 2024

Describe the bug
aiokafka.structs.BrokerMetadata#nodeId is typed as an int, however it is sometimes assigned str values:

  • It is assigned a str here
  • Also assigned a str here
  • It is assigned an int here (the value is passed in to add_coordinator(...) here, the value of resp.coordinator_id is an integer, as defined here, and here)

Expected behaviour
aiokafka.structs.BrokerMetadata#nodeId is correctly typed as a typing.Union[int, str]

Value reads do not seem to care too much about the type of this field, as long as:

  • It is unique
  • It can be used as a dict key

So no further changes should be needed

Alternative is to make it a str field, and convert int to str where int values are currently being assigned.

Environment (please complete the following information):
N/A this is a typing bug, I generally use Pyright to type check.

Additional observations
Also note that the name nodeId is camel-case, whereas Python prefers (and the rest of the code-base are) snake-case. I will provide a PR to resolve the above issue shortly. I am happy to rename nodeId to node_id as well, if that is desired, but I do not plan to, to minimize the changes introduced by the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant