From 08282c28169ec8a3046257193055fd200e102d1a Mon Sep 17 00:00:00 2001 From: Sarang Sawant Date: Thu, 25 Aug 2022 21:30:16 -0600 Subject: [PATCH] CORTX-33151 Codacy fixes related to style and security Solution: a) Fix all pep257 issues. b) False positive and conflicting Codacy rules are disabled in codacy flake8. c) Fix medium priority and security issues. d) Fix 13 test cases in classes TestDeliveryHeraldAny & TestDeliveryHeraldAll. Signed-off-by: Sarang Sawant --- cfgen/cfgen | 2 +- ha-simulator/docker/Dockerfile | 1 + hax/hax/common/__init__.py | 10 +- hax/hax/ha/message_type/__init__.py | 3 +- hax/hax/ha/message_type/message_type.py | 1 + hax/hax/ha/resource/__init__.py | 2 +- hax/hax/handler.py | 20 +-- hax/hax/message.py | 1 + hax/hax/motr/rconfc.py | 1 + hax/hax/queue/__init__.py | 10 ++ hax/hax/server.py | 5 +- hax/helper/configure.py | 17 ++- hax/helper/exec.py | 7 + hax/helper/generate_sysconf.py | 2 + hax/test/integration/test_server.py | 2 + hax/test/integration/testutils.py | 1 + hax/test/test_delivery_herald.py | 131 +++++++++--------- hax/test/test_failure.py | 6 +- hax/test/test_work_planner.py | 15 +- .../miniprov/hare_mp/consul_starter.py | 2 +- provisioning/miniprov/hare_mp/hax_starter.py | 10 +- provisioning/miniprov/hare_mp/main.py | 10 +- provisioning/miniprov/hare_mp/types.py | 2 + provisioning/miniprov/hare_mp/utils.py | 10 +- provisioning/miniprov/hare_mp/validator.py | 1 + provisioning/miniprov/test/__init__.py | 3 + provisioning/miniprov/test/test_cdf.py | 15 +- utils/utils.py | 4 +- 28 files changed, 169 insertions(+), 125 deletions(-) diff --git a/cfgen/cfgen b/cfgen/cfgen index 5572e5a4a..25b397076 100755 --- a/cfgen/cfgen +++ b/cfgen/cfgen @@ -380,7 +380,7 @@ def validate_cdf_schema(cdf: str, cdf_path: str, schema_path: str) -> None: input=cdf) except CliException as e: die( - f'{cdf_path}: Invalid cluster description\n' + e.stderr + + f"{cdf_path}: Invalid cluster description\n" + e.stderr + f"\n\nTo learn more, run '{sys.argv[0]} --help-schema'.") diff --git a/ha-simulator/docker/Dockerfile b/ha-simulator/docker/Dockerfile index 0f32cad99..cc6fb826b 100644 --- a/ha-simulator/docker/Dockerfile +++ b/ha-simulator/docker/Dockerfile @@ -17,6 +17,7 @@ COPY ./patches/ /app/patches/ RUN python3 -m venv --system-site-packages .py-venv/ \ && source ./.py-venv/bin/activate \ && pip install -r ./requirements.txt +# shellcheck disable=SC2046 RUN pip3 install -r https://raw.githubusercontent.com/Seagate/cortx-utils/main/py-utils/python_requirements.txt \ && pip3 install -r https://raw.githubusercontent.com/Seagate/cortx-utils/main/py-utils/python_requirements.ext.txt \ && yum install -y http://cortx-storage.colo.seagate.com/releases/cortx/github/main/centos-7.9.2009/last_successful/$(curl --silent http://cortx-storage.colo.seagate.com/releases/cortx/github/main/centos-7.9.2009/last_successful/ | grep py-utils | sed 's/.*href="\([^"]*\)".*/\1/g') diff --git a/hax/hax/common/__init__.py b/hax/hax/common/__init__.py index 5be341efe..e338facba 100644 --- a/hax/hax/common/__init__.py +++ b/hax/hax/common/__init__.py @@ -15,15 +15,15 @@ # For any questions about this software or licensing, # please email opensource@seagate.com or cortx-questions@seagate.com. # +"""Hax commons package symbols.""" import inject class HaxGlobalState: - """ - Global state of whole HaX application. - """ + """Global state of whole Hax application.""" def __init__(self): + """Initialize current state to non-stopping state.""" self.stopping: bool = False def is_stopping(self) -> bool: @@ -36,7 +36,5 @@ def set_stopping(self): def di_configuration(binder: inject.Binder): - """ - Configures Dependency Injection (DI) engine. - """ + """Configures Dependency Injection (DI) engine.""" binder.bind(HaxGlobalState, HaxGlobalState()) diff --git a/hax/hax/ha/message_type/__init__.py b/hax/hax/ha/message_type/__init__.py index a0f2b6595..44726bee1 100644 --- a/hax/hax/ha/message_type/__init__.py +++ b/hax/hax/ha/message_type/__init__.py @@ -16,4 +16,5 @@ # please email opensource@seagate.com or cortx-questions@seagate.com. # -# empty +"""Module provides Message type interface and Health Message implementation to +send health events to message bus topic `cortx_health_events`. """ diff --git a/hax/hax/ha/message_type/message_type.py b/hax/hax/ha/message_type/message_type.py index 922f504a4..908a7c0c1 100644 --- a/hax/hax/ha/message_type/message_type.py +++ b/hax/hax/ha/message_type/message_type.py @@ -22,6 +22,7 @@ class MessageType(): def __init__(self): + """Message base class with abstract send method.""" logging.debug('Inside MessageType') def send(self, event, util: ConsulUtil): diff --git a/hax/hax/ha/resource/__init__.py b/hax/hax/ha/resource/__init__.py index a0f2b6595..399b3fe74 100644 --- a/hax/hax/ha/resource/__init__.py +++ b/hax/hax/ha/resource/__init__.py @@ -16,4 +16,4 @@ # please email opensource@seagate.com or cortx-questions@seagate.com. # -# empty +"""Modue provides Resource Type interface to send resource status events.""" diff --git a/hax/hax/handler.py b/hax/hax/handler.py index d058ddb6b..6509b9fe2 100644 --- a/hax/hax/handler.py +++ b/hax/hax/handler.py @@ -326,7 +326,7 @@ def _restart_notify(self, req: FirstEntrypointRequest, self.consul.set_proc_restart_count(req.process_fid, restart_count + 1) - def _do_work(self, planner: WorkPlanner, motr: Motr): + def _do_work(self, planner: WorkPlanner, motr: Motr): # noqa: MC0001 LOG.info('Handler thread has started') try: @@ -340,14 +340,14 @@ def _do_work(self, planner: WorkPlanner, motr: Motr): if isinstance(item, FirstEntrypointRequest): self._restart_notify(item, motr) motr.send_entrypoint_request_reply( - EntrypointRequest( - reply_context=item.reply_context, - req_id=item.req_id, - remote_rpc_endpoint=item.remote_rpc_endpoint, - process_fid=item.process_fid, - git_rev=item.git_rev, - pid=item.pid, - is_first_request=item.is_first_request)) + EntrypointRequest( + reply_context=item.reply_context, + req_id=item.req_id, + remote_rpc_endpoint=item.remote_rpc_endpoint, + process_fid=item.process_fid, + git_rev=item.git_rev, + pid=item.pid, + is_first_request=item.is_first_request)) elif isinstance(item, EntrypointRequest): # While replying any Exception is catched. In such a # case, the motr process will receive EAGAIN and @@ -368,7 +368,7 @@ def _do_work(self, planner: WorkPlanner, motr: Motr): motr.broadcast_ha_states(item.states, update_kv=False) status = self.consul.objHealthToProcessEvent( - item.states[0].status) + item.states[0].status) # Account for received process status. self.consul.update_process_status_local( ConfHaProcess(chp_event=status, diff --git a/hax/hax/message.py b/hax/hax/message.py index 7813db9cb..f5b6b5b37 100644 --- a/hax/hax/message.py +++ b/hax/hax/message.py @@ -76,6 +76,7 @@ class HaNvecGetEvent(BaseMessage): nvec: List[HaNote] def __repr__(self): + """Machine readable string representation of HaNvecGetEvent.""" return f'HaNvecGetEvent(<{len(self.nvec)} items>)' diff --git a/hax/hax/motr/rconfc.py b/hax/hax/motr/rconfc.py index e60def4a2..3a2d8373a 100644 --- a/hax/hax/motr/rconfc.py +++ b/hax/hax/motr/rconfc.py @@ -20,6 +20,7 @@ class RconfcStarter(StoppableThread): can happen when Motr is initialized in hax). """ def __init__(self, motr: Motr, consul_util: ConsulUtil): + """Rconfc starter's thread constructor.""" super().__init__(target=self._execute, name='rconfc-starter', args=(motr, )) diff --git a/hax/hax/queue/__init__.py b/hax/hax/queue/__init__.py index 6eb67d9b4..3c6dd5322 100644 --- a/hax/hax/queue/__init__.py +++ b/hax/hax/queue/__init__.py @@ -1,3 +1,4 @@ +"""Broadcast Queue Consumer implementation.""" import json import logging from queue import Queue @@ -27,6 +28,15 @@ class BQProcessor: """ def __init__(self, planner: WorkPlanner, delivery_herald: DeliveryHerald, motr: Motr, conf_obj_util: ConfObjUtil): + """Initialize pre requisites for Broadcast Queue Processor. + + Args: + planner: work planner for Motr-aware threads (see ConsumerThread). + delivery_herald: Thread synchronizing block that ensures + delivery is confirmed. + motr: hax side foreign fucntion interface wrapper. + conf_obj_util: references KV(consul) adapter. + """ self.planner = planner self.confobjutil = conf_obj_util self.herald = delivery_herald diff --git a/hax/hax/server.py b/hax/hax/server.py index bf1ed25d9..399f22b6b 100644 --- a/hax/hax/server.py +++ b/hax/hax/server.py @@ -106,7 +106,8 @@ def hctl_stat(request): # request a parameter. The respective result is obtained by executing # corresponding `hctl-fetch-fids` script. def hctl_fetch_fids(request): - """calls the hare-fetch-fids to provide info about services configured. + """ + calls the hare-fetch-fids to provide info about services configured. This function calls the hare-fetch-fids script from the hax-server in order to provide details about services configured by Hare e.g. Hax, @@ -124,7 +125,7 @@ def hctl_fetch_fids(request): def to_ha_states(data: Any, consul_util: ConsulUtil) -> List[HAState]: """ - converts dictionary into list of HA states + converts dictionary into list of HA states. Converts a dictionary, obtained from JSON data, into a list of HA states. diff --git a/hax/helper/configure.py b/hax/helper/configure.py index 115696d4f..aecc7fd80 100644 --- a/hax/helper/configure.py +++ b/hax/helper/configure.py @@ -103,8 +103,8 @@ def _setup_logging(opts: AppCtx): @click.pass_context def parse_opts(ctx, cdf: str, conf_dir: str, transport: str, log_dir: str, consul_server: bool, uuid: str, log_file: str): - """Generate Hare configuration according to the given CDF file. - + """ + Generate Hare configuration according to the given CDF file. CDF Full path to the Cluster Description File (CDF). """ ctx.ensure_object(dict) @@ -119,7 +119,20 @@ def parse_opts(ctx, cdf: str, conf_dir: str, transport: str, log_dir: str, class ConfGenerator: + + """Generates confd.dhall from CDF file and uses m0confgen to generate motr + specific confd.xc. + + confd.dhall is a pre-configuration file for Motr using dhall configuration + language which is an input to dhall utility that generates a list of motr + configuration objects corresponding to motr configuration obt structures + in a human readable format. This output is then passed as an input to + m0confgen to generate motr specific `confd.xc` that follows + motr xcode grammar. + """ + def __init__(self, context: AppCtx): + """Construct ConfGenerator with necessary context.""" self.cdf_path = context.cdf_path self.conf_dir = context.conf_dir self.transport = context.transport diff --git a/hax/helper/exec.py b/hax/helper/exec.py index 0e62ece80..f6144c98a 100644 --- a/hax/helper/exec.py +++ b/hax/helper/exec.py @@ -28,6 +28,12 @@ class Program: to be executed by Executor class. """ def __init__(self, cmd: List[str], stdin: Optional['Program'] = None): + """Holds a command or chain of commands. + + Args: + cmd: a single command. + stdin: input to the command. + """ self.cmd = cmd self.stdin = stdin @@ -38,6 +44,7 @@ def __or__(self, other): return other def __repr__(self): + """Machine readable repr of the command""" return f'Program({self.cmd}, stdin={self.stdin})' diff --git a/hax/helper/generate_sysconf.py b/hax/helper/generate_sysconf.py index 5226d00ad..76d3e09d8 100755 --- a/hax/helper/generate_sysconf.py +++ b/hax/helper/generate_sysconf.py @@ -49,6 +49,7 @@ class KVFile: """Helper to fetch data from Hare-Motr configuration key values file.""" def __init__(self, kv_file: str, node: str) -> None: + """Loads Hare-Motr configuration in memory to generate sysconfig.""" self.kv_file = kv_file self.kv_data = self._read_file() self.node = node @@ -127,6 +128,7 @@ class ConsulKV: """ def __init__(self, node: str) -> None: + """Initializes the Consul KV adapter.""" self.kv = KVAdapter() self.node = node diff --git a/hax/test/integration/test_server.py b/hax/test/integration/test_server.py index 14b5875ca..9dfc32ce1 100644 --- a/hax/test/integration/test_server.py +++ b/hax/test/integration/test_server.py @@ -125,9 +125,11 @@ def __init__(self, pattern: List[HAState]): self.pattern = pattern def __repr__(self): + """Machine readable representation of list of HA states""" return str(self.pattern) def __eq__(self, value): + """Equality support for BroadcastHAStates.""" if not isinstance(value, BroadcastHAStates): return False return self.pattern == value.states diff --git a/hax/test/integration/testutils.py b/hax/test/integration/testutils.py index 3bc69da0e..65d2d7cbe 100644 --- a/hax/test/integration/testutils.py +++ b/hax/test/integration/testutils.py @@ -217,6 +217,7 @@ def fn(trace: Invocation) -> bool: return fn +# pylint: disable=E0202 class FakeFFI(HaxFFI): def __init__(self): self.traces: List[Invocation] = [] diff --git a/hax/test/test_delivery_herald.py b/hax/test/test_delivery_herald.py index 6be8adeb2..43614dee3 100644 --- a/hax/test/test_delivery_herald.py +++ b/hax/test/test_delivery_herald.py @@ -22,6 +22,7 @@ import unittest from threading import Condition, Thread from time import sleep, time +from queue import Queue from hax.exception import NotDelivered from hax.log import TRACE @@ -73,17 +74,17 @@ def setUpClass(cls): def test_it_works(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=1) - def fn(): + def fn(notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(MessageId(halink_ctx=100, tag=1)) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) - t = Thread(target=fn) + t = Thread(target=fn, args=(notified_ok,)) t.start() m = MessageId @@ -91,22 +92,22 @@ def fn(): [m(100, 1), m(100, 3), m(100, 4)]), timeout_sec=10) t.join() - self.assertTrue(notified_ok, + self.assertTrue(not notified_ok.qsize(), 'Unexpected exception appeared in notifier thread') def test_exception_raised_by_timeout(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=1) - def fn(): + def fn(notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(MessageId(halink_ctx=43, tag=3)) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) - t = Thread(target=fn) + t = Thread(target=fn, args=(notified_ok,)) t.start() m = MessageId @@ -117,23 +118,23 @@ def fn(): timeout_sec=5) finally: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_works_under_load(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=32) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(msg) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i), )) + Thread(target=fn, args=(MessageId(100, i), notified_ok)) for i in range(1, 32) ] for t in threads: @@ -149,16 +150,16 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_if_delivered_earlier_than_awaited_wait_works(self): herald = DeliveryHerald() - notified_ok = True thread_count = 1 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -168,10 +169,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -189,17 +190,17 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertEqual(0, len(herald.unsorted_deliveries.keys())) def test_if_delivered_earlier_than_awaited_works_immediately(self): herald = DeliveryHerald() - notified_ok = True thread_count = 1 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -209,10 +210,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -232,7 +233,7 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertLess( finished - started, 5, @@ -242,11 +243,11 @@ def m(x): def test_if_delivered_earlier_than_awaited_wait_many(self): herald = DeliveryHerald() - notified_ok = True thread_count = 6 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -256,10 +257,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -278,7 +279,7 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertEqual(4, len(herald.unsorted_deliveries.keys())) @@ -296,38 +297,38 @@ def setUpClass(cls): def test_it_works(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=1) - def fn(): + def fn(notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(MessageId(halink_ctx=100, tag=1)) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) - t = Thread(target=fn) + t = Thread(target=fn, args=(notified_ok,)) t.start() m = MessageId herald.wait_for_all(HaLinkMessagePromise([m(100, 1)]), timeout_sec=5) t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_exception_raised_if_not_all_delivered(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=1) - def fn(): + def fn(notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(MessageId(halink_ctx=42, tag=3)) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) - t = Thread(target=fn) + t = Thread(target=fn, args=(notified_ok,)) t.start() m = MessageId @@ -339,23 +340,23 @@ def fn(): finally: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_works_if_all_messages_confirmed(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=1) - def fn(): + def fn(notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(MessageId(halink_ctx=42, tag=3)) herald.notify_delivered(MessageId(halink_ctx=42, tag=1)) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) - t = Thread(target=fn) + t = Thread(target=fn, args=(notified_ok,)) t.start() m = MessageId @@ -365,23 +366,23 @@ def fn(): timeout_sec=5) finally: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_works_under_load(self): herald = DeliveryHerald() - notified_ok = True + notified_ok = Queue(maxsize=32) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: sleep(1.5) herald.notify_delivered(msg) except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i), )) + Thread(target=fn, args=(MessageId(100, i), notified_ok)) for i in range(1, 32) ] for t in threads: @@ -397,16 +398,16 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') def test_if_delivered_earlier_than_awaited_wait_works(self): herald = DeliveryHerald() - notified_ok = True thread_count = 1 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -416,10 +417,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -437,17 +438,17 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertEqual(0, len(herald.unsorted_deliveries.keys())) def test_if_delivered_earlier_than_awaited_wait_many(self): herald = DeliveryHerald() - notified_ok = True thread_count = 6 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -457,10 +458,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -479,17 +480,17 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertEqual(4, len(herald.unsorted_deliveries.keys())) def test_if_delivered_earlier_than_awaited_notified_immediately(self): herald = DeliveryHerald() - notified_ok = True thread_count = 1 + notified_ok = Queue(maxsize=thread_count) latch = CountDownLatch(thread_count) - def fn(msg: MessageId): + def fn(msg: MessageId, notified_ok: Queue): try: LOG.debug('Thread started') herald.notify_delivered(msg) @@ -499,10 +500,10 @@ def fn(msg: MessageId): except: logging.exception('*** ERROR ***') - notified_ok = False + notified_ok.put(False) threads = [ - Thread(target=fn, args=(MessageId(100, i + 1), )) + Thread(target=fn, args=(MessageId(100, i + 1), notified_ok)) for i in range(thread_count) ] @@ -523,7 +524,7 @@ def m(x): finally: for t in threads: t.join() - self.assertTrue(notified_ok, + self.assertTrue(notified_ok.qsize() == 0, 'Unexpected exception appeared in notifier thread') self.assertLess( finished - started, 5, diff --git a/hax/test/test_failure.py b/hax/test/test_failure.py index bfd76e441..bfb4b5bf0 100644 --- a/hax/test/test_failure.py +++ b/hax/test/test_failure.py @@ -171,17 +171,17 @@ def test_drive_failure(self): herald.wait_for_any = Mock() bqprocessor = BQProcessor(planner, herald, motr, consul_util) - + svc_name = 'hax' drive_fid = Fid(0x6b00000000000001, 0x11) hax_fid = Fid(0x7200000000000001, 0x6) - + def fake_add(cmd): if isinstance(cmd, BroadcastHAStates): motr.broadcast_ha_states(cmd.states, kv_cache=consul_cache) if hasattr(cmd, 'reply_to') and cmd.reply_to: cmd.reply_to.put([MessageId(0, 0)]) - + planner.add_command = Mock(side_effect=fake_add) payload = { diff --git a/hax/test/test_work_planner.py b/hax/test/test_work_planner.py index 23dd408bf..6bbef46d7 100644 --- a/hax/test/test_work_planner.py +++ b/hax/test/test_work_planner.py @@ -246,7 +246,6 @@ def fn(planner: WorkPlanner, exc: Queue): def test_entrypoint_request_processed_asap(self): planner = WorkPlanner() - group_idx = 0 tracker = TimeTracker() thread_count = 4 planner.add_command(broadcast()) @@ -257,10 +256,9 @@ def test_entrypoint_request_processed_asap(self): for j in range(thread_count): planner.add_command(Die()) - exc = None + excq = Queue(maxsize=thread_count) - def fn(planner: WorkPlanner): - nonlocal exc + def fn(planner: WorkPlanner, exc: Queue): try: while True: LOG.log(TRACE, "Requesting for a work") @@ -282,18 +280,19 @@ def fn(planner: WorkPlanner): except Exception as e: LOG.exception('*** ERROR ***') - exc = e + exc.put(e) workers = [ - Thread(target=fn, args=(planner, )) for t in range(thread_count) + Thread(target=fn, args=(planner, excq)) for t in range(thread_count) ] for t in workers: t.start() for t in workers: t.join() - if exc: - raise exc + # raises the first collected exception and test can be improved + if excq.qsize() != 0: + raise excq.get() tracks = tracker.get_tracks() cmd = tracks[0][0] self.assertTrue(isinstance(cmd, EntrypointRequest)) diff --git a/provisioning/miniprov/hare_mp/consul_starter.py b/provisioning/miniprov/hare_mp/consul_starter.py index f3c16545e..e578fe050 100644 --- a/provisioning/miniprov/hare_mp/consul_starter.py +++ b/provisioning/miniprov/hare_mp/consul_starter.py @@ -38,7 +38,7 @@ def __init__(self, utils: Utils, cns_utils: ConsulUtil, stop_event: Event, log_dir: str, data_dir: str, config_dir: str, node_name: str, node_id: str, peers: List[str], - bind_addr: str = '0.0.0.0', + bind_addr: str = '0.0.0.0', # nosec client_addr: str = '0.0.0.0'): super().__init__(target=self._execute, name='consul-starter') diff --git a/provisioning/miniprov/hare_mp/hax_starter.py b/provisioning/miniprov/hare_mp/hax_starter.py index 787d737e3..f5784085e 100644 --- a/provisioning/miniprov/hare_mp/hax_starter.py +++ b/provisioning/miniprov/hare_mp/hax_starter.py @@ -30,11 +30,15 @@ class HaxStarter(StoppableThread): - """ - Starts consul agent and blocks until terminated. - """ + + """Starts consul agent and blocks until terminated.""" def __init__(self, utils: Utils, stop_event: Event, home_dir: str, log_dir: str): + """Starts hax and hence consul agent. + + Arguments: + process -- forms the hax command + """ super().__init__(target=self._execute, name='hax-starter') self.utils = utils diff --git a/provisioning/miniprov/hare_mp/main.py b/provisioning/miniprov/hare_mp/main.py index 616989065..da62d0c36 100644 --- a/provisioning/miniprov/hare_mp/main.py +++ b/provisioning/miniprov/hare_mp/main.py @@ -178,10 +178,10 @@ def logrotate_generic(url: str): @func_log(func_enter, func_leave) def logrotate(url: str): - ''' This function is kept incase needed in future. - This function configures logrotate based on - 'setup_type' key from confstore - ''' + """This function is kept incase needed in future. + This function configures logrotate based on + 'setup_type' key from confstore + """ try: server_type = get_server_type(url) logging.info('Server type (%s)', server_type) @@ -908,7 +908,7 @@ def list2dict( for node in nodes_data_hctl: node_svc_info: Dict[str, List[str]] = {} for service in node['svcs']: - if not service['name'] in node_svc_info.keys(): + if not service['name'] in node_svc_info: node_svc_info[service['name']] = [] if (service['status'] == 'started'): node_svc_info[service['name']].append(service['status']) diff --git a/provisioning/miniprov/hare_mp/types.py b/provisioning/miniprov/hare_mp/types.py index 0bddf4ed7..badb28156 100644 --- a/provisioning/miniprov/hare_mp/types.py +++ b/provisioning/miniprov/hare_mp/types.py @@ -29,6 +29,7 @@ class DList(Sequence[T]): comment: str def __getitem__(self, ndx): + """Allows using [] operator.""" return self.value[ndx] def __len__(self): @@ -67,6 +68,7 @@ def v(field): for k in fields(self)) + ' }' def __repr__(self): + """Machine readable str representation of the DhallTuple object.""" return self.__str__() diff --git a/provisioning/miniprov/hare_mp/utils.py b/provisioning/miniprov/hare_mp/utils.py index 48b998c3a..002801001 100755 --- a/provisioning/miniprov/hare_mp/utils.py +++ b/provisioning/miniprov/hare_mp/utils.py @@ -42,9 +42,7 @@ def func_enter(func): - """ - Logs function entry point. - """ + """Logs function entry point.""" func_name = func.__qualname__ func_line = func.__code__.co_firstlineno func_filename = func.__code__.co_filename @@ -53,9 +51,7 @@ def func_enter(func): def func_leave(func): - """ - Logs function exit point. - """ + """Logs function exit point.""" logging.info('Leaving %s', func.__qualname__) @@ -73,6 +69,7 @@ def call(*args, **kwargs): class Utils: def __init__(self, provider: ValueProvider): + """Initializes value provider like ConfStore.""" self.provider = provider self.kv = KVAdapter() @@ -393,6 +390,7 @@ def get_node_group(self, machine_id: str, allow_null: bool = False): class LogWriter: def __init__(self, logger: logging.Logger, logging_handler): + """Initialize LogWriter object with Logger and LoggingHandler obj.""" self.logger = logger self.logging_handler = logging_handler diff --git a/provisioning/miniprov/hare_mp/validator.py b/provisioning/miniprov/hare_mp/validator.py index f8dbd70ea..3f2eb571c 100644 --- a/provisioning/miniprov/hare_mp/validator.py +++ b/provisioning/miniprov/hare_mp/validator.py @@ -20,6 +20,7 @@ class Validator: def __init__(self, provider: ValueProvider): + """Initializes value provider like conf store.""" super().__init__() self.provider = provider diff --git a/provisioning/miniprov/test/__init__.py b/provisioning/miniprov/test/__init__.py index 3faaab116..a78cb4370 100644 --- a/provisioning/miniprov/test/__init__.py +++ b/provisioning/miniprov/test/__init__.py @@ -15,6 +15,9 @@ # For any questions about this software or licensing, # please email opensource@seagate.com or cortx-questions@seagate.com. # + +"""Mini provisioning test module.""" + import inject from hax.common import di_configuration diff --git a/provisioning/miniprov/test/test_cdf.py b/provisioning/miniprov/test/test_cdf.py index c8471612e..95a98a7ff 100644 --- a/provisioning/miniprov/test/test_cdf.py +++ b/provisioning/miniprov/test/test_cdf.py @@ -29,9 +29,8 @@ from hare_mp.cdf import CdfGenerator from hare_mp.store import ConfStoreProvider, ValueProvider -from hare_mp.types import (DisksDesc, Disk, DList, M0ClientDesc, M0ServerDesc, Maybe, - MissingKeyError, PoolDesc, PoolType, Protocol, Text, - AllowedFailures, Layout) +from hare_mp.types import (DisksDesc, DList, M0ClientDesc, M0ServerDesc, Maybe, + PoolDesc, PoolType, Protocol, Text, Layout) from hare_mp.utils import Utils from hax.util import KVAdapter @@ -223,7 +222,6 @@ def ret_values(value: str) -> Any: 'tcp', 'cortx>motr>client_instances': 2, - 'node>MACH_ID>num_cvg': 1, 'cluster>num_storage_set': 1, 'cluster>storage_set>server_node_count': @@ -585,7 +583,6 @@ def new_kv(key: str, val: str): def ret_values(value: str) -> Any: data = { - 'node>MACH_ID>num_cvg': 1, 'cluster>num_storage_set': 1, 'cluster>storage_set>server_node_count': 1, 'cluster>storage_set[0]>name': 'StorageSet-1', @@ -670,10 +667,10 @@ def my_get(key: str, recurse: bool = False, allow_null: bool = False): cdf.generate() def test_invalid_storage_set_configuration_rejected(self): - ''' This test case checks whether exception will be raise if total - number of data devices are less than - data_units + parity_units + spare_units - ''' + """This test case checks whether exception will be raise + if total number of data devices are less than + data_units + parity_units + spare_units. + """ store = ValueProvider() def ret_values(value: str) -> Any: diff --git a/utils/utils.py b/utils/utils.py index b72bc4567..c1528fd3f 100755 --- a/utils/utils.py +++ b/utils/utils.py @@ -186,11 +186,11 @@ def consul_is_active_at(hostname: str) -> bool: def pcs_consul_is_active_at(hostname: str) -> bool: cmd = ssh_prefix(hostname) + \ 'sudo systemctl is-active --quiet hare-consul-agent*' - return subprocess.call(cmd, shell=True) == 0 + return subprocess.call(cmd, shell=True) == 0 # nosec def exec_silent(cmd: str) -> bool: - return subprocess.call(cmd, shell=True) == 0 + return subprocess.call(cmd, shell=True) == 0 # nosec def exec_custom(cmd: str, show_err: bool = True) -> None: