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

Update group.partitions_for_topic to fetch topic metadata if it does … #1781

Merged
merged 1 commit into from
May 23, 2019

Conversation

Baisang
Copy link
Contributor

@Baisang Baisang commented Apr 7, 2019

…not have it

As mentioned in #1774, this attempts to simulate the Java client, which does the following

public java.util.List<PartitionInfo> partitionsFor(java.lang.String topic)
Get metadata about the partitions for a given topic. This method will issue a remote call to the server if it does not already have any metadata about the given topic.

https://kafka.apache.org/10/javadoc/index.html?org/apache/kafka/clients/producer/KafkaProducer.html#partitionsFor


This change is Reviewable

Copy link
Collaborator

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

This looks really close to me with just a few small tweaks...

If you can fixup / answer the questions, then I'd be happy to merge as its clearly affecting a bunch of people.

kafka/consumer/group.py Outdated Show resolved Hide resolved
kafka/consumer/group.py Show resolved Hide resolved
kafka/consumer/group.py Outdated Show resolved Hide resolved

Returns:
set: topics
def _fetch_all_topic_metadata(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the Java client structured this way with this private method?

It's a nice clean way to do it this way, but just curious how they handle it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kafka/consumer/group.py Outdated Show resolved Hide resolved
kafka/consumer/group.py Outdated Show resolved Hide resolved
@Baisang Baisang force-pushed the baisang/partitions_for_topic branch from 097a7c0 to 1878cc0 Compare May 23, 2019 06:17
@Baisang
Copy link
Contributor Author

Baisang commented May 23, 2019

@jeffwidman Thanks for the feedback -- I addressed your comments, hopefully travis is kind to me.

@jeffwidman jeffwidman merged commit 1f73287 into dpkp:master May 23, 2019
@jeffwidman
Copy link
Collaborator

🎆

Thanks again for this!

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