From 99a677faca12aa17e150d6ec85e3ef3e4e19f0b0 Mon Sep 17 00:00:00 2001 From: TheByronHimes Date: Fri, 28 Jul 2023 11:42:45 +0000 Subject: [PATCH] Use warnings instead of errors in load/delete --- mass/core/query_handler.py | 27 ++++++++++++++------------- mass/ports/inbound/query_handler.py | 20 ++++++++++++++++---- tests/test_consumer.py | 13 ++++++++----- tests/test_resources.py | 15 ++++++++------- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/mass/core/query_handler.py b/mass/core/query_handler.py index 170e34d..0d75d56 100644 --- a/mass/core/query_handler.py +++ b/mass/core/query_handler.py @@ -14,6 +14,7 @@ # limitations under the License. # """Contains implementation of a QueryHandler to field queries on metadata""" +import warnings from typing import Optional from hexkit.custom_types import JsonObject @@ -23,7 +24,8 @@ from mass.core import models from mass.ports.inbound.query_handler import ( ClassNotConfiguredError, - DeletionFailedError, + ClassNotConfiguredWarning, + DeletionFailedWarning, QueryHandlerPort, SearchError, ) @@ -48,22 +50,21 @@ def __init__( async def load_resource(self, *, resource: models.Resource, class_name: str): if class_name not in self._config.searchable_classes: - raise ClassNotConfiguredError(class_name=class_name) - - dao = self._dao_collection.get_dao(class_name=class_name) - - await dao.upsert(resource) + warnings.warn(message=class_name, category=ClassNotConfiguredWarning) + else: + dao = self._dao_collection.get_dao(class_name=class_name) + await dao.upsert(resource) async def delete_resource(self, *, resource_id: str, class_name: str): if class_name not in self._config.searchable_classes: - raise ClassNotConfiguredError(class_name=class_name) + warnings.warn(message=class_name, category=ClassNotConfiguredWarning) + else: + dao = self._dao_collection.get_dao(class_name=class_name) - dao = self._dao_collection.get_dao(class_name=class_name) - - try: - await dao.delete(id_=resource_id) - except ResourceNotFoundError as err: - raise DeletionFailedError(resource_id) from err + try: + await dao.delete(id_=resource_id) + except ResourceNotFoundError: + warnings.warn(message=resource_id, category=DeletionFailedWarning) async def handle_query( self, diff --git a/mass/ports/inbound/query_handler.py b/mass/ports/inbound/query_handler.py index 782c01d..027cedb 100644 --- a/mass/ports/inbound/query_handler.py +++ b/mass/ports/inbound/query_handler.py @@ -29,6 +29,18 @@ def __init__(self, class_name: str): super().__init__(message) +class ClassNotConfiguredWarning(RuntimeWarning): + """Raised when loaded/deleted resource's class_name isn't configured + + Used for load/delete functions instead of ClassNotFiguredError in order to + avoid stopping the event consumer. + """ + + def __init__(self, class_name: str): + message = f"Class with name '{class_name}' not configured." + super().__init__(message) + + class SearchError(RuntimeError): """Raised when there is a problem searching with the query parameters.""" @@ -38,7 +50,7 @@ def __init__(self): ) -class DeletionFailedError(RuntimeError): +class DeletionFailedWarning(RuntimeWarning): """Raised when a deletion attempt fails because the ID can't be found""" def __init__(self, resource_id: str): @@ -56,9 +68,9 @@ async def delete_resource(self, *, resource_id: str, class_name: str): """Delete resource with given ID and class name from the database Raises: - ClassNotConfiguredError - when the class_name parameter does not + ClassNotConfiguredWarning - when the class_name parameter does not match any configured class - DeletionFailedError - when the provided ID doesn't match any resource + DeletionFailedWarning - when the provided ID doesn't match any resource found in the database. """ @@ -91,6 +103,6 @@ async def load_resource( """Load a resource into the database. Raises: - ClassNotConfiguredError - when the class_name parameter does not match + ClassNotConfiguredWarning - when the class_name parameter does not match any configured class. """ diff --git a/tests/test_consumer.py b/tests/test_consumer.py index f1978e7..4f66947 100644 --- a/tests/test_consumer.py +++ b/tests/test_consumer.py @@ -19,7 +19,7 @@ from ghga_event_schemas import pydantic_ as event_schemas from mass.core import models -from mass.ports.inbound.query_handler import DeletionFailedError +from mass.ports.inbound.query_handler import DeletionFailedWarning from tests.fixtures.joint import JointFixture @@ -59,7 +59,7 @@ async def test_resource_upsert( # put together event payload payload = event_schemas.SearchableResource( - accession=resource.id_, + accession=resource_id, class_name="DatasetEmbedded", content=content, ).dict() @@ -69,7 +69,7 @@ async def test_resource_upsert( payload=payload, type_=joint_fixture.config.resource_upsertion_event_type, topic=joint_fixture.config.searchable_resource_events_topic, - key=f"dataset_embedded_{resource.id_}", + key=f"dataset_embedded_{resource_id}", ) # consume the event @@ -128,7 +128,10 @@ async def test_resource_delete(joint_fixture: JointFixture): @pytest.mark.asyncio async def test_delete_nonexistent_resource(joint_fixture: JointFixture): - """Test for correct error handling when trying to delete a non-existent resource""" + """Test for correct warning when trying to delete a non-existent resource + + Don't want to stop event consumer with an error, so should only create a warning + """ query_handler = await joint_fixture.container.query_handler() @@ -153,6 +156,6 @@ async def test_delete_nonexistent_resource(joint_fixture: JointFixture): ) # consume the event - with pytest.raises(DeletionFailedError): + with pytest.warns(DeletionFailedWarning): consumer = await joint_fixture.container.event_subscriber() await consumer.run(forever=False) diff --git a/tests/test_resources.py b/tests/test_resources.py index 4270d6f..5837ec8 100644 --- a/tests/test_resources.py +++ b/tests/test_resources.py @@ -21,7 +21,8 @@ from mass.core import models from mass.ports.inbound.query_handler import ( ClassNotConfiguredError, - DeletionFailedError, + ClassNotConfiguredWarning, + DeletionFailedWarning, SearchError, ) from tests.fixtures.config import get_config @@ -193,7 +194,7 @@ async def test_resource_load(joint_fixture: JointFixture): @pytest.mark.asyncio async def test_loading_non_configured_resource(joint_fixture: JointFixture): - """Test that we get the right error for loading a non-configured class_name""" + """Test that we get the right warning for loading a non-configured class_name""" query_handler = await joint_fixture.container.query_handler() # define and load a new resource @@ -205,7 +206,7 @@ async def test_loading_non_configured_resource(joint_fixture: JointFixture): "category": "test object", }, ) - with pytest.raises(ClassNotConfiguredError): + with pytest.warns(ClassNotConfiguredWarning): await query_handler.load_resource(resource=resource, class_name="ThisWillBreak") @@ -272,7 +273,7 @@ async def test_resource_deletion(joint_fixture: JointFixture): @pytest.mark.asyncio async def test_resource_deletion_failure(joint_fixture: JointFixture): - """Test for correct errors when failing to delete a resource""" + """Test for correct warning when failing to delete a resource""" query_handler = await joint_fixture.container.query_handler() all_resources = await query_handler.handle_query( @@ -282,7 +283,7 @@ async def test_resource_deletion_failure(joint_fixture: JointFixture): assert all_resources.count > 0 # try to delete a resource that doesn't exist - with pytest.raises(DeletionFailedError): + with pytest.warns(DeletionFailedWarning): await query_handler.delete_resource( resource_id="not-here", class_name="DatasetEmbedded" ) @@ -297,10 +298,10 @@ async def test_resource_deletion_failure(joint_fixture: JointFixture): @pytest.mark.asyncio async def test_resource_deletion_not_configured(joint_fixture: JointFixture): - """Test for correct error when trying to delete a non-configured resource""" + """Test for correct warning when trying to delete a non-configured resource""" query_handler = await joint_fixture.container.query_handler() - with pytest.raises(ClassNotConfiguredError): + with pytest.warns(ClassNotConfiguredWarning): await query_handler.delete_resource( resource_id="1HotelAlpha-id", class_name="Not-Configured" )