Skip to content

Commit

Permalink
Use warnings instead of errors in load/delete
Browse files Browse the repository at this point in the history
  • Loading branch information
TheByronHimes committed Jul 28, 2023
1 parent ef58b2f commit 99a677f
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 29 deletions.
27 changes: 14 additions & 13 deletions mass/core/query_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,7 +24,8 @@
from mass.core import models
from mass.ports.inbound.query_handler import (
ClassNotConfiguredError,
DeletionFailedError,
ClassNotConfiguredWarning,
DeletionFailedWarning,
QueryHandlerPort,
SearchError,
)
Expand All @@ -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,
Expand Down
20 changes: 16 additions & 4 deletions mass/ports/inbound/query_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand All @@ -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):
Expand All @@ -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.
"""

Expand Down Expand Up @@ -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.
"""
13 changes: 8 additions & 5 deletions tests/test_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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()

Expand All @@ -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)
15 changes: 8 additions & 7 deletions tests/test_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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")


Expand Down Expand Up @@ -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(
Expand All @@ -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"
)
Expand All @@ -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"
)

0 comments on commit 99a677f

Please sign in to comment.