From 461a885eeb85d3ec2f23d0759dddfd1012e5b021 Mon Sep 17 00:00:00 2001 From: vitor-aignosi Date: Mon, 20 Oct 2025 16:48:21 -0300 Subject: [PATCH] SIENTIAPDE-1312 Refactor OPC handling by removing pod_id from initialization and updating logging format - Removed pod_id parameter from OPC class and repository initialization to streamline connection management. - Updated logging statements for improved readability during disconnection attempts and error handling. --- laborious/activities/opc.py | 1 - laborious/activities/storage.py | 2 +- laborious/utils/repository/opc_repository.py | 26 +++++----- tests/laborious/activities/test_opc.py | 2 - .../utils/repository/test_opc_repository.py | 51 ++++++++++++++++--- 5 files changed, 60 insertions(+), 22 deletions(-) diff --git a/laborious/activities/opc.py b/laborious/activities/opc.py index b4604d8..1951973 100644 --- a/laborious/activities/opc.py +++ b/laborious/activities/opc.py @@ -85,7 +85,6 @@ class OPC(BaseActivity): server_cert_path=server['server_cert_path'], notification_handler=self.notification_handler, reconnection_interval=server['reconnection_interval'], - pod_id=self.pod_id, ) is_connected, error_data = await self.opc_repository[opc_id].connect() if not is_connected: diff --git a/laborious/activities/storage.py b/laborious/activities/storage.py index 5439420..c04b6a4 100644 --- a/laborious/activities/storage.py +++ b/laborious/activities/storage.py @@ -10,7 +10,7 @@ with workflow.unsafe.imports_passed_through(): from sientia_do.notifications.models import NotificationLevel from sientia_do.observability.logger import Logger from sientia_do.temporal.activities.postgres import Postgres - from sientia_do.temporal.constants import now, DATETIME_FORMAT_FILENAME + from sientia_do.temporal.constants import DATETIME_FORMAT_FILENAME, now from laborious.utils.repository.minio_repository import MinioRepository diff --git a/laborious/utils/repository/opc_repository.py b/laborious/utils/repository/opc_repository.py index d53cf1b..8ab8563 100644 --- a/laborious/utils/repository/opc_repository.py +++ b/laborious/utils/repository/opc_repository.py @@ -45,14 +45,13 @@ class OpcRepository(BaseActivity): self, opc_id: str, url: str, - pod_id: str, logger: Logger, notification_handler: NotificationHandler, reconnection_interval: int = 60, server_uri: str | None = None, cert_path: str | None = None, private_key_path: str | None = None, - server_cert_path: str | None = None + server_cert_path: str | None = None, ): self.url = url self.id = opc_id @@ -66,7 +65,6 @@ class OpcRepository(BaseActivity): self.last_reconnection_time: None | datetime = None self.notification_handler = notification_handler self.client: None | Client = None - self.pod_id = pod_id BaseActivity.__init__(self, logger, notification_handler, set_error_counter=True) @@ -192,16 +190,20 @@ class OpcRepository(BaseActivity): error_stack = [] for i in range(5): try: - self.logger.info(f'Disconnecting from OPC UA server, attempt {i+1} of 5') + self.logger.info(f'Disconnecting from OPC UA server, attempt {i + 1} of 5') await self.client.disconnect() return [] except Exception as e: - self.logger.error(f'Failed to disconnect from OPC UA server in attempt {i+1} of 5: {e}') - error_stack.append({ - 'attempt': i+1, - 'error': str(e), - 'traceback': traceback.format_exc(), - }) + self.logger.error( + f'Failed to disconnect from OPC UA server in attempt {i + 1} of 5: {e}' + ) + error_stack.append( + { + 'attempt': i + 1, + 'error': str(e), + 'traceback': traceback.format_exc(), + } + ) await asyncio.sleep(0.1 * i) return error_stack @@ -221,13 +223,13 @@ class OpcRepository(BaseActivity): self.send_notification( metadata=self.metadata, notification_id=f'OPC_DISCONNECTION_ERROR_{self.id}', - message=f'Failed to disconnect from OPC server in 5 attempts: {errors}', + message='Failed to disconnect from OPC server in 5 attempts.', block='opc_repository', level=NotificationLevel.ERROR, attachment_content=json.dumps(errors, indent=4), ) else: - self.logger.warning(f'Disconnected from OPC server {self.id} successfully') + self.logger.warning(f'Disconnected from OPC server {self.id} successfully') self.client = None diff --git a/tests/laborious/activities/test_opc.py b/tests/laborious/activities/test_opc.py index 815df27..a9bbe67 100644 --- a/tests/laborious/activities/test_opc.py +++ b/tests/laborious/activities/test_opc.py @@ -105,7 +105,6 @@ async def test_init_opc(mock_send_notification, mock_opc_repository): server_cert_path='', notification_handler=mock_notification_handler, reconnection_interval=60, - pod_id='localhost', ), ] ) @@ -121,7 +120,6 @@ async def test_init_opc(mock_send_notification, mock_opc_repository): server_cert_path='', notification_handler=mock_notification_handler, reconnection_interval=60, - pod_id='localhost', ) ] ) diff --git a/tests/laborious/utils/repository/test_opc_repository.py b/tests/laborious/utils/repository/test_opc_repository.py index 6ec680d..2a47f13 100644 --- a/tests/laborious/utils/repository/test_opc_repository.py +++ b/tests/laborious/utils/repository/test_opc_repository.py @@ -1,3 +1,4 @@ +import json from datetime import datetime from unittest.mock import ANY, AsyncMock, MagicMock, Mock, call, patch @@ -15,7 +16,7 @@ def mock_logger(): @pytest.fixture def opc_repository(mock_logger): - return OpcRepository( + repository = OpcRepository( opc_id='test_repo', url='opc.tcp://localhost:4840', logger=mock_logger, @@ -26,6 +27,8 @@ def opc_repository(mock_logger): private_key_path='/path/to/key.pem', server_cert_path='/path/to/server_cert.pem', ) + repository.send_notification = MagicMock() + return repository @pytest.fixture @@ -162,11 +165,37 @@ async def test_try_connect_no_client(opc_repository): @pytest.mark.asyncio -async def test_disconnect(opc_repository, mock_client): +async def test_disconnection_fallback_success(opc_repository, mock_client): opc_repository.client = mock_client - await opc_repository.disconnect() + mock_client.disconnect.return_value = True + result = await opc_repository.disconnection_fallback() mock_client.disconnect.assert_called_once() + assert result == [] + + +@pytest.mark.asyncio +async def test_disconnection_fallback_fail(opc_repository, mock_client): + opc_repository.client = mock_client + mock_client.disconnect.side_effect = Exception('Test error') + result = await opc_repository.disconnection_fallback() + assert result == [ + {'attempt': 1, 'error': 'Test error', 'traceback': ANY}, + {'attempt': 2, 'error': 'Test error', 'traceback': ANY}, + {'attempt': 3, 'error': 'Test error', 'traceback': ANY}, + {'attempt': 4, 'error': 'Test error', 'traceback': ANY}, + {'attempt': 5, 'error': 'Test error', 'traceback': ANY}, + ] + assert mock_client.disconnect.call_count == 5 + + +@pytest.mark.asyncio +async def test_disconnect(opc_repository, mock_client): + opc_repository.client = mock_client + opc_repository.disconnection_fallback = AsyncMock(return_value=[]) + await opc_repository.disconnect() + + opc_repository.disconnection_fallback.assert_called_once() assert opc_repository.client is None @@ -179,11 +208,21 @@ async def test_disconnect_no_client(opc_repository): @pytest.mark.asyncio async def test_disconnect_error(opc_repository, mock_client): opc_repository.client = mock_client - mock_client.disconnect.side_effect = Exception('Test error') + opc_repository.disconnection_fallback = AsyncMock( + return_value=[{'attempt': 1, 'error': 'Test error', 'traceback': 'text'}] + ) await opc_repository.disconnect() - opc_repository.logger.custom_error.assert_called_once_with( - 'Failed to disconnect from OPC server: Test error', ANY + opc_repository.disconnection_fallback.assert_called_once() + opc_repository.send_notification.assert_called_once_with( + metadata=opc_repository.metadata, + notification_id=f'OPC_DISCONNECTION_ERROR_{opc_repository.id}', + message='Failed to disconnect from OPC server in 5 attempts.', + block='opc_repository', + level=NotificationLevel.ERROR, + attachment_content=json.dumps( + [{'attempt': 1, 'error': 'Test error', 'traceback': 'text'}], indent=4 + ), ) assert opc_repository.client is None