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

Misc updates #169

Merged
merged 1 commit into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions bubuku/broker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from time import time, sleep

from bubuku.config import KafkaProperties
from bubuku.id_generator import BrokerIdGenerator
from bubuku.id_extractor import BrokerIdExtractor
from bubuku.process import KafkaProcess
from bubuku.zookeeper import BukuExhibitor

Expand Down Expand Up @@ -76,7 +76,7 @@ def on_timeout_fail(self):

class BrokerManager(object):
def __init__(self, process: KafkaProcess, exhibitor: BukuExhibitor,
id_manager: BrokerIdGenerator, kafka_properties: KafkaProperties, timeout: StartupTimeout):
id_manager: BrokerIdExtractor, kafka_properties: KafkaProperties, timeout: StartupTimeout):
self.id_manager = id_manager
self.exhibitor = exhibitor
self.kafka_properties = kafka_properties
Expand Down
6 changes: 3 additions & 3 deletions bubuku/env_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import requests

from bubuku.config import Config, KafkaProperties
from bubuku.id_generator import BrokerIdGenerator
from bubuku.id_extractor import BrokerIdExtractor
from bubuku.zookeeper import BukuExhibitor, AddressListProvider
from bubuku.zookeeper.exhibitor import ExhibitorAddressProvider

Expand Down Expand Up @@ -86,7 +86,7 @@ def get_address_provider(self):
return ExhibitorAddressProvider(partial(self._load_instance_ips, self.config.zk_stack_name))

def create_broker_id_manager(self, zk: BukuExhibitor, kafka_props: KafkaProperties):
return BrokerIdGenerator(zk, kafka_props)
return BrokerIdExtractor(zk, kafka_props)


class _LocalAddressProvider(AddressListProvider):
Expand Down Expand Up @@ -117,4 +117,4 @@ def get_rack(self):
return None

def create_broker_id_manager(self, zk: BukuExhibitor, kafka_props: KafkaProperties):
return BrokerIdGenerator(zk, kafka_props)
return BrokerIdExtractor(zk, kafka_props)
20 changes: 11 additions & 9 deletions bubuku/id_generator.py → bubuku/id_extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,22 @@
import logging
import os
import re
from typing import Optional, List

from bubuku.config import KafkaProperties
from bubuku.zookeeper import BukuExhibitor

_LOG = logging.getLogger('bubuku.id_generator')


class BrokerIdGenerator(object):
def _search_broker_id(lines: List[str]) -> Optional[str]:
for line in lines:
match = re.search('^broker\\.id=(\\d+)$', line.strip())
if match:
return match.group(1)


class BrokerIdExtractor(object):
def __init__(self, zk: BukuExhibitor, kafka_properties: KafkaProperties):
super().__init__()
self.zk = zk
Expand All @@ -24,17 +32,11 @@ def get_broker_id(self):
while not os.path.isfile(meta_path):
return None
with open(meta_path) as f:
lines = f.readlines()
for line in lines:
match = re.search('broker\.id=(\d+)', line)
if match:
self.broker_id = match.group(1)
return self.broker_id
return None
self.broker_id = _search_broker_id(f.readlines())
return self.broker_id
Copy link
Contributor

Choose a reason for hiding this comment

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

will it return None as before in case there is no match ?

Copy link
Contributor

Choose a reason for hiding this comment

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

i see tests 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as it did before. I was trying to understand the logic behind it. Failed and renamed the class.


def is_registered(self):
broker_id = self.get_broker_id()
if broker_id:
return self.zk.is_broker_registered(broker_id)
return False

2 changes: 1 addition & 1 deletion delivery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pipeline:
type: script
overlay: guild-python/legacy
env:
PYENV_VERSION: 3.6.10
PYENV_VERSION: 3.8.10
commands:
- desc: Install Docker
cmd: |
Expand Down
20 changes: 20 additions & 0 deletions tests/test_broker_id_generator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import unittest

from bubuku.id_extractor import _search_broker_id


class TestBrokerIdExtractor(unittest.TestCase):
def test_match_valid(self):
assert '123534' == _search_broker_id(['broker.id=123534'])
assert '123534' == _search_broker_id(['\tbroker.id=123534'])
assert '123534' == _search_broker_id(['\tbroker.id=123534\n'])
assert '123534' == _search_broker_id(['broker.id=123534 \n\r'])
assert '123534' == _search_broker_id(['\tbroker.id=123534 \r'])
assert '123534' == _search_broker_id(['xbroker.id=1', 'broker.id=123534'])
assert '123534' == _search_broker_id(['broker.id=123534', 'boker.id=123534'])

def test_match_invalid(self):
assert _search_broker_id([]) is None
assert _search_broker_id(['broker_id=123534']) is None
assert _search_broker_id(['xbroker.id=1', 'broker.id=12f3534']) is None
assert _search_broker_id(['bruker.id=123534', 'boker.id=123534']) is None
4 changes: 2 additions & 2 deletions tests/test_check_time_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@


def test_check_time_period():
test_check = TestCheck()
test_check = _TestCheck()

assert test_check.check_if_time() is not None # first time it should always run
assert test_check.check_if_time() is None # time has not come yet
Expand All @@ -14,7 +14,7 @@ def test_check_time_period():
assert 0.0 < test_check.time_till_check() < 1 # there's still some time before the check can be run again


class TestCheck(Check):
class _TestCheck(Check):
def __init__(self):
super().__init__(check_interval_s=0.5)

Expand Down
10 changes: 5 additions & 5 deletions tests/test_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from test_config import build_test_properties


class TestController(object):
class _TestController(object):
def __init__(self):
self.checks = []

Expand All @@ -18,7 +18,7 @@ def test_load_restart_on_exhibitor():
exhibitor = object()
broker = object()

controller = TestController()
controller = _TestController()

apply_features(-1, {'restart_on_exhibitor': {}}, controller, exhibitor, broker, None, None)

Expand All @@ -33,7 +33,7 @@ def test_rebalance_on_start():
exhibitor = object()
broker = object()

controller = TestController()
controller = _TestController()

apply_features(-1, {'rebalance_on_start': {}}, controller, exhibitor, broker, None, None)

Expand All @@ -49,7 +49,7 @@ def test_rebalance_on_broker_list_change():
exhibitor = object()
broker = object()

controller = TestController()
controller = _TestController()

apply_features(-1, {'rebalance_on_brokers_change': {}}, controller, exhibitor, broker, None, None)

Expand All @@ -67,7 +67,7 @@ def test_graceful_terminate():

broker = object()

controller = TestController()
controller = _TestController()

apply_features(-1, {'graceful_terminate': {}}, controller, None, broker, None, None)

Expand Down