-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Ignore MetadataResponses with empty broker list #1506
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
Conversation
test/test_cluster.py
Outdated
from kafka.cluster import ClusterMetadata | ||
from kafka.protocol.metadata import MetadataResponse | ||
|
||
import kafka.common as Errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing... why import common as Errors? Why not directly import errors as errors or common as common?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! I removed that line as it is not needed. But generally re: kafka.common -- that line was copy-pasta from a file that was written a long time ago before kafka.common was split into kafka.errors and kafka.structs . The only reason to keep kafka.common is to prevent breaking user code that was written before the change. But for our library code we should use kafka.errors directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I thought... I finished the library cleanup here: #1509 so hopefully this won't happen again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Fix for #1494 . The MetadataResponse should be ignored and treated as a failure if the broker list is empty (same as the java client).