From f780008283b823386205b5c64830abbba9d58200 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Tue, 4 Jul 2023 15:36:54 +0200 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Nicola Coretti --- .../peer_communicator/peer_is_ready_sender.py | 17 ++++++++++++----- .../peer_communicator/sender.py | 6 +++++- .../peer_communication/analyze_log.py | 6 +++--- .../test_background_peer_state.py | 18 +++++++++++------- .../test_peer_is_ready_sender.py | 1 - .../test_synchronize_connection_sender.py | 2 +- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/exasol_advanced_analytics_framework/udf_communication/peer_communicator/peer_is_ready_sender.py b/exasol_advanced_analytics_framework/udf_communication/peer_communicator/peer_is_ready_sender.py index 5036a0ff..cd40f81e 100644 --- a/exasol_advanced_analytics_framework/udf_communication/peer_communicator/peer_is_ready_sender.py +++ b/exasol_advanced_analytics_framework/udf_communication/peer_communicator/peer_is_ready_sender.py @@ -11,7 +11,15 @@ LOGGER: FilteringBoundLogger = structlog.get_logger() +import enum + class PeerIsReadySender: + + class State(enum.IntFlag) + Init = enum.auto() + Enabled = enum.auto() + Finished = enum.auto() + def __init__(self, out_control_socket: Socket, peer: Peer, @@ -20,15 +28,14 @@ def __init__(self, self._timer = timer self._peer = peer self._out_control_socket = out_control_socket - self._finished = False - self._enabled = False + self._state = State.Init self._logger = LOGGER.bind( peer=self._peer.dict(), my_connection_info=my_connection_info.dict()) def enable(self): self._logger.debug("enable") - self._enabled = True + self._state |= State.Enabled def reset_timer(self): self._logger.debug("reset_timer") @@ -38,12 +45,12 @@ def send_if_necessary(self, force=False): self._logger.debug("send_if_necessary") should_we_send = self._should_we_send() if should_we_send or force: - self._finished = True + self._state |= State.Finished self._send_peer_is_ready_to_frontend() def _should_we_send(self): is_time = self._timer.is_time() - result = is_time and not self._finished and self._enabled + result = is_time and (State.Finished not in self._state) and (State.Enabled in self._state) return result def _send_peer_is_ready_to_frontend(self): diff --git a/exasol_advanced_analytics_framework/udf_communication/peer_communicator/sender.py b/exasol_advanced_analytics_framework/udf_communication/peer_communicator/sender.py index 55d8e703..41acbd3d 100644 --- a/exasol_advanced_analytics_framework/udf_communication/peer_communicator/sender.py +++ b/exasol_advanced_analytics_framework/udf_communication/peer_communicator/sender.py @@ -34,7 +34,11 @@ def create_send_socket(self) -> Socket: try: send_socket = self._socket_factory.create_socket(SocketType.DEALER) send_socket.connect( - f"tcp://{self._peer.connection_info.ipaddress.ip_address}:{self._peer.connection_info.port.port}") + f"tcp://{ip}:{port}".format( + ip=self._peer.connection_info.ipaddress.ip_address, + port=self._peer.connection_info.port.port + ) + return send_socket except Exception: send_socket.close() diff --git a/tests/udf_communication/peer_communication/analyze_log.py b/tests/udf_communication/peer_communication/analyze_log.py index a1437b25..7f514053 100644 --- a/tests/udf_communication/peer_communication/analyze_log.py +++ b/tests/udf_communication/peer_communication/analyze_log.py @@ -12,15 +12,15 @@ def is_log_sequence_ok(lines: List[Dict[str, str]], line_predicate: Callable[[Di return result -def is_peer_is_ready_send(line: Dict[str, str]): +def is_peer_ready_to_send(line: Dict[str, str]): return line["module"] == "peer_is_ready_sender" and line["event"] == "send" -def is_received_acknowledge_connection(line: Dict[str, str]): +def is_connection_acknowledged(line: Dict[str, str]): return line["module"] == "background_peer_state" and line["event"] == "received_acknowledge_connection" -def is_received_synchronize_connection(line: Dict[str, str]): +def is_connection_synchronized(line: Dict[str, str]): return line["module"] == "background_peer_state" and line["event"] == "received_synchronize_connection" diff --git a/tests/udf_communication/peer_communication/test_background_peer_state.py b/tests/udf_communication/peer_communication/test_background_peer_state.py index a56035ed..46b43787 100644 --- a/tests/udf_communication/peer_communication/test_background_peer_state.py +++ b/tests/udf_communication/peer_communication/test_background_peer_state.py @@ -36,13 +36,17 @@ class TestSetup: synchronize_connection_sender_mock: Union[MagicMock, SynchronizeConnectionSender] background_peer_state: BackgroundPeerState - def reset_mock(self): - self.abort_timeout_sender_mock.reset_mock() - self.synchronize_connection_sender_mock.reset_mock() - self.peer_is_ready_sender_mock.reset_mock() - self.sender_mock.reset_mock() - self.receive_socket_mock.reset_mock() - self.socket_factory_mock.reset_mock() + def reset_mocks(self): + mocks = ( + self.abort_timeout_sender_mock, + self.synchronize_connection_sender_mock, + self.peer_is_ready_sender_mock, + self.sender_mock, + self.receive_socket_mock, + self.socket_factory_mock, + ) + for mock in mocks: + mock.reset_mock() def create_test_setup() -> TestSetup: diff --git a/tests/udf_communication/peer_communication/test_peer_is_ready_sender.py b/tests/udf_communication/peer_communication/test_peer_is_ready_sender.py index 1b5e7256..5c3412dd 100644 --- a/tests/udf_communication/peer_communication/test_peer_is_ready_sender.py +++ b/tests/udf_communication/peer_communication/test_peer_is_ready_sender.py @@ -163,7 +163,6 @@ def test_send_if_necessary_after_enable_and_is_time_true_twice(): def test_reset_timer(): test_setup = create_test_setup() - print(test_setup.timer_mock.mock_calls) test_setup.peer_is_ready_sender.reset_timer() assert ( test_setup.out_control_socket_mock.mock_calls == [] diff --git a/tests/udf_communication/peer_communication/test_synchronize_connection_sender.py b/tests/udf_communication/peer_communication/test_synchronize_connection_sender.py index 38d9ead9..be73c99b 100644 --- a/tests/udf_communication/peer_communication/test_synchronize_connection_sender.py +++ b/tests/udf_communication/peer_communication/test_synchronize_connection_sender.py @@ -21,7 +21,7 @@ class TestSetup: sender_mock: Union[MagicMock, Sender] synchronize_connection_sender: SynchronizeConnectionSender - def reset_mock(self): + def reset_mocks(self): self.sender_mock.reset_mock() self.timer_mock.reset_mock()