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

Fix the return value of Cluster.findTopicPartitionMetadata #749

Merged

Conversation

ankon
Copy link
Contributor

@ankon ankon commented Jun 8, 2020

The return value is not an Object, but an Array (of PartitionMetadata objects). This also means we can
simply check the number of entries and don't need to calculate the keys when sending messages.

Note that this exposed a bunch of problems in the tests as well: the mocked calls not just returned the
wrong type, but in fact returned a completely unrelated value.

@@ -30,7 +30,7 @@ module.exports = ({ logger, cluster, partitioner, eosManager }) => {
for (const { topic, messages } of topicMessages) {
const partitionMetadata = cluster.findTopicPartitionMetadata(topic)

if (keys(partitionMetadata).length === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that this works is kind of a funny Javascript-ism. 🙈

@Nevon
Copy link
Collaborator

Nevon commented Jun 30, 2020

Thanks. If you fix the conflict I'll merge this.

@ankon ankon force-pushed the pr/find-topic-partition-metadata-type branch from 7da0fa9 to 901dd7f Compare July 3, 2020 09:48
@ankon
Copy link
Contributor Author

ankon commented Jul 3, 2020

Turns out I didn't split another PR properly, and the actual change in the type was already merged with 78fefe2, so this PR is only updating the users to match as well.

(Rebased onto current master)

@Nevon
Copy link
Collaborator

Nevon commented Jul 20, 2020

And now this needs a merge as well.

The return value is not an Object, but an Array (of PartitionMetadata objects). This means we can
simply check the number of entries and don't need to calculate the keys when sending messages. Similarly
tests need to use the correct arrays rather than some unrelated values.
@ankon ankon force-pushed the pr/find-topic-partition-metadata-type branch from 901dd7f to df1d820 Compare July 20, 2020 16:28
@ankon
Copy link
Contributor Author

ankon commented Jul 20, 2020

There you go :)

@Nevon Nevon merged commit b922b9e into tulios:master Jul 20, 2020
@ankon ankon deleted the pr/find-topic-partition-metadata-type branch July 20, 2020 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants