Skip to content

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 26, 2018
Merged

Ignore MetadataResponses with empty broker list #1506

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79d731f
Empty broker list in metadata should be ignored
dpkp May 25, 2018
a8f566a
Ignore MetadataResponse if broker list is empty
dpkp May 25, 2018
b55d049
remove unneded import in test_cluster.py
dpkp May 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion kafka/cluster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ def update_metadata(self, metadata):
return self.failed_update(error)

if not metadata.brokers:
log.warning("No broker metadata found in MetadataResponse")
log.warning("No broker metadata found in MetadataResponse -- ignoring.")
return self.failed_update(Errors.MetadataEmptyBrokerList(metadata))

_new_brokers = {}
for broker in metadata.brokers:
Expand Down
4 changes: 4 additions & 0 deletions kafka/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ class StaleMetadata(KafkaError):
invalid_metadata = True


class MetadataEmptyBrokerList(KafkaError):
retriable = True


class UnrecognizedBrokerVersion(KafkaError):
pass

Expand Down
24 changes: 24 additions & 0 deletions test/test_cluster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# pylint: skip-file
from __future__ import absolute_import

import pytest

from kafka.cluster import ClusterMetadata
from kafka.protocol.metadata import MetadataResponse

import kafka.common as Errors
Copy link
Contributor

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?

Copy link
Owner Author

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.

Copy link
Contributor

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...



def test_empty_broker_list():
cluster = ClusterMetadata()
assert len(cluster.brokers()) == 0

cluster.update_metadata(MetadataResponse[0](
[(0, 'foo', 12), (1, 'bar', 34)], []))
assert len(cluster.brokers()) == 2

# empty broker list response should be ignored
cluster.update_metadata(MetadataResponse[0](
[], # empty brokers
[(17, 'foo', []), (17, 'bar', [])])) # topics w/ error
assert len(cluster.brokers()) == 2