From 6d672d674595cb6d6050f6ccaa19d9d699a6b01d Mon Sep 17 00:00:00 2001 From: Piotr Dyba Date: Thu, 4 Aug 2022 12:55:21 +0200 Subject: [PATCH 1/4] At the moment, we have two classes that represent events that we are using for asynchronous communication between services: BaseEvent and Event. After implementing them, it turned out that they are very similar and it is possible to use only one instead. The biggest benefit is simplicity as having one class is always easier to understand/use. --- CHANGELOG.md | 7 ++++++ Makefile | 4 +++ lbz/events/api.py | 43 ++++++++++---------------------- lbz/events/broker.py | 18 ++++++------- lbz/events/event.py | 26 +++++++++++++++++++ requirements-dev.txt | 8 +++--- setup.cfg | 4 +-- tests/test_events/test_api.py | 42 +++++-------------------------- tests/test_events/test_broker.py | 7 +++--- tests/test_events/test_event.py | 36 ++++++++++++++++++++++++++ version | 2 +- 11 files changed, 113 insertions(+), 84 deletions(-) create mode 100644 lbz/events/event.py create mode 100644 tests/test_events/test_event.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 42e2300..01cb91a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -264,3 +264,10 @@ Released 2022-08-01 - Adds missing exception Too Early - 425 +### Version 0.5.12 +Released 2022-08-05 + +- Use the same class representing Event (both sent/received) +- Adds additional verbose test running to makefile +- Updates dev dependancies + diff --git a/Makefile b/Makefile index 7035308..c36b846 100644 --- a/Makefile +++ b/Makefile @@ -88,6 +88,10 @@ test-unit tu: .PHONY: test test: test-unit +.PHONY: test-unit-verbose tuv +test-unit-verbose tuv: + coverage run --include "lbz/*" -m pytest "tests" -vv + coverage report -m --skip-covered ############################################################################### # Custom Scripts diff --git a/lbz/events/api.py b/lbz/events/api.py index 1987e0e..56a9d97 100644 --- a/lbz/events/api.py +++ b/lbz/events/api.py @@ -1,10 +1,10 @@ -import json from copy import deepcopy from functools import wraps from os import getenv from typing import TYPE_CHECKING, Any, Callable, List from lbz.aws_boto3 import client +from lbz.events.event import Event from lbz.misc import Singleton, get_logger if TYPE_CHECKING: @@ -18,30 +18,13 @@ MAX_EVENTS_TO_SEND_AT_ONCE = 10 -class BaseEvent: - type: str - - def __init__(self, raw_data: dict) -> None: - self.raw_data = raw_data - self.data: str = self.serialize(raw_data) - - def __eq__(self, other: object) -> bool: - if isinstance(other, BaseEvent): - return self.type == other.type and self.raw_data == other.raw_data - return False - - @staticmethod - def serialize(raw_data: dict) -> str: - return json.dumps(raw_data, default=str) - - class EventAPI(metaclass=Singleton): def __init__(self) -> None: self._source = getenv("AWS_LAMBDA_FUNCTION_NAME") or "lbz-event-api" self._resources: List[str] = [] - self._pending_events: List[BaseEvent] = [] - self._sent_events: List[BaseEvent] = [] - self._failed_events: List[BaseEvent] = [] + self._pending_events: List[Event] = [] + self._sent_events: List[Event] = [] + self._failed_events: List[Event] = [] self._bus_name = getenv("EVENTS_BUS_NAME", f"{self._source}-event-bus") def __repr__(self) -> str: @@ -60,28 +43,28 @@ def set_bus_name(self, bus_name: str) -> None: self._bus_name = bus_name @property - def sent_events(self) -> List[BaseEvent]: + def sent_events(self) -> List[Event]: return deepcopy(self._sent_events) @property - def pending_events(self) -> List[BaseEvent]: + def pending_events(self) -> List[Event]: return deepcopy(self._pending_events) @property - def failed_events(self) -> List[BaseEvent]: + def failed_events(self) -> List[Event]: return deepcopy(self._failed_events) - def register(self, new_event: BaseEvent) -> None: + def register(self, new_event: Event) -> None: self._pending_events.append(new_event) # TODO: Stop sharing protected lists outside the class, use the above properties instead - def get_all_pending_events(self) -> List[BaseEvent]: + def get_all_pending_events(self) -> List[Event]: return self._pending_events - def get_all_sent_events(self) -> List[BaseEvent]: + def get_all_sent_events(self) -> List[Event]: return self._sent_events - def get_all_failed_events(self) -> List[BaseEvent]: + def get_all_failed_events(self) -> List[Event]: return self._failed_events def send(self) -> None: @@ -108,9 +91,9 @@ def clear(self) -> None: self._pending_events = [] self._failed_events = [] - def _create_eb_entry(self, new_event: BaseEvent) -> PutEventsRequestEntryTypeDef: + def _create_eb_entry(self, new_event: Event) -> PutEventsRequestEntryTypeDef: return { - "Detail": new_event.data, + "Detail": new_event.serialized_data, "DetailType": new_event.type, "EventBusName": self._bus_name, "Resources": self._resources, diff --git a/lbz/events/broker.py b/lbz/events/broker.py index d964ebb..dddf1fc 100644 --- a/lbz/events/broker.py +++ b/lbz/events/broker.py @@ -1,21 +1,21 @@ -from dataclasses import dataclass from typing import Callable, Dict, List +from lbz.events.event import Event from lbz.misc import get_logger logger = get_logger(__name__) -@dataclass() -class Event: - type: str - data: dict - - class EventBroker: - def __init__(self, mapper: Dict[str, List[Callable]], raw_event: dict) -> None: + def __init__( + self, + mapper: Dict[str, List[Callable]], + raw_event: dict, + type_key: str = "detail-type", + data_key: str = "detail", + ) -> None: self.mapper = mapper - self.event = Event(type=raw_event["detail-type"], data=raw_event["detail"]) + self.event = Event(raw_event[data_key], event_type=raw_event[type_key]) def handle(self) -> None: self.pre_handle() diff --git a/lbz/events/event.py b/lbz/events/event.py new file mode 100644 index 0000000..64dbcca --- /dev/null +++ b/lbz/events/event.py @@ -0,0 +1,26 @@ +import json +from typing import Optional + + +class Event: + type: str + + def __init__(self, data: dict, *, event_type: Optional[str] = None) -> None: + self.data = data + self.type = event_type or self.type + + def __eq__(self, other: object) -> bool: + if isinstance(other, Event): + return self.type == other.type and self.data == other.data + return False + + def __repr__(self) -> str: + return f"Event(type='{self.type}', data={self.data})" + + @staticmethod + def serialize(raw_data: dict) -> str: + return json.dumps(raw_data, default=str) + + @property + def serialized_data(self) -> str: + return self.serialize(self.data) diff --git a/requirements-dev.txt b/requirements-dev.txt index 77fa901..3574f9b 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -12,7 +12,7 @@ black==22.6.0 # via -r requirements-dev.in boto3-stubs[cognito-idp,dynamodb,events,lambda,s3,sns,sqs,ssm]==1.21.46 # via -r requirements-dev.in -botocore-stubs==1.27.42.post2 +botocore-stubs==1.27.45 # via boto3-stubs build==0.8.0 # via pip-tools @@ -26,7 +26,7 @@ coverage[toml]==6.4.2 # pytest-cov dill==0.3.5.1 # via pylint -flake8==5.0.0 +flake8==5.0.4 # via -r requirements-dev.in iniconfig==1.1.1 # via pytest @@ -80,7 +80,7 @@ pluggy==1.0.0 # via pytest py==1.11.0 # via pytest -pycodestyle==2.9.0 +pycodestyle==2.9.1 # via flake8 pyflakes==2.5.0 # via flake8 @@ -104,6 +104,8 @@ tomli==2.0.1 # pytest tomlkit==0.11.1 # via pylint +types-awscrt==0.13.14 + # via botocore-stubs typing-extensions==4.3.0 # via # astroid diff --git a/setup.cfg b/setup.cfg index 7c18420..bdd25aa 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,8 +24,8 @@ use_parentheses = True [mypy] # https://mypy.readthedocs.io/en/latest/running_mypy.html#mapping-file-paths-to-modules explicit_package_bases = True -mypy_path = - $MYPY_CONFIG_FILE_DIR, +;mypy_path = +; $MYPY_CONFIG_FILE_DIR, namespace_packages = True # https://mypy.readthedocs.io/en/latest/config_file.html diff --git a/tests/test_events/test_api.py b/tests/test_events/test_api.py index 24464ac..f404d1c 100644 --- a/tests/test_events/test_api.py +++ b/tests/test_events/test_api.py @@ -6,44 +6,14 @@ from pytest import LogCaptureFixture from lbz.aws_boto3 import Boto3Client -from lbz.events.api import BaseEvent, EventAPI, event_emitter +from lbz.events.api import EventAPI, event_emitter +from lbz.events.event import Event -class MyTestEvent(BaseEvent): +class MyTestEvent(Event): type = "MY_TEST_EVENT" -class TestBaseEvent: - def test_base_event_creation_and_structure(self) -> None: - event = {"x": 1} - new_event = MyTestEvent(event) - - assert new_event.type == "MY_TEST_EVENT" - assert new_event.raw_data == {"x": 1} - assert new_event.data == '{"x": 1}' - - def test__eq__same(self) -> None: - new_event_1 = MyTestEvent({"x": 1}) - new_event_2 = MyTestEvent({"x": 1}) - - assert new_event_1 == new_event_2 - - def test__eq__different_data(self) -> None: - new_event_1 = MyTestEvent({"x": 1}) - new_event_2 = MyTestEvent({"x": 2}) - - assert new_event_1 != new_event_2 - - def test__eq__different_type_same_data(self) -> None: - class MySecondTestEvent(BaseEvent): - type = "MY_SECOND_TEST_EVENT" - - new_event_1 = MyTestEvent({"x": 1}) - new_event_2 = MySecondTestEvent({"x": 1}) - - assert new_event_1 != new_event_2 - - class TestEventAPI: def setup_method(self) -> None: # pylint: disable= attribute-defined-outside-init @@ -270,18 +240,18 @@ def decorated_function() -> None: def test_sends_all_pending_events_when_decorated_function_finished_with_success(self) -> None: @event_emitter def decorated_function() -> None: - EventAPI().register(MyTestEvent(raw_data={"x": 1})) + EventAPI().register(MyTestEvent({"x": 1})) decorated_function() - assert EventAPI().sent_events == [MyTestEvent(raw_data={"x": 1})] + assert EventAPI().sent_events == [MyTestEvent({"x": 1})] assert not EventAPI().pending_events assert not EventAPI().failed_events def test_clears_queues_when_error_appeared_during_running_decorated_function(self) -> None: @event_emitter def decorated_function() -> None: - EventAPI().register(MyTestEvent(raw_data={"x": 1})) + EventAPI().register(MyTestEvent({"x": 1})) raise RuntimeError with pytest.raises(RuntimeError): diff --git a/tests/test_events/test_broker.py b/tests/test_events/test_broker.py index 35b2312..838e5cf 100644 --- a/tests/test_events/test_broker.py +++ b/tests/test_events/test_broker.py @@ -4,14 +4,15 @@ import pytest from pytest import LogCaptureFixture -from lbz.events.broker import Event, EventBroker +from lbz.events.broker import EventBroker +from lbz.events.event import Event class TestEventBroker: def test_broker_works_properly(self) -> None: func_1 = MagicMock() func_2 = MagicMock() - expected_event = Event(type="x", data={"y": 1}) + expected_event = Event({"y": 1}, event_type="x") mapper = {"x": [func_1, func_2]} event = {"detail-type": "x", "detail": {"y": 1}} @@ -36,7 +37,7 @@ def test_broker_continues_even_if_one_handler_failed(self, caplog: LogCaptureFix func_1 = MagicMock() func_2 = MagicMock(side_effect=TypeError) func_3 = MagicMock() - expected_event = Event(type="x", data={"y": 1}) + expected_event = Event({"y": 1}, event_type="x") mapper = {"x": [func_1, func_2, func_3]} event = {"detail-type": "x", "detail": {"y": 1}} diff --git a/tests/test_events/test_event.py b/tests/test_events/test_event.py new file mode 100644 index 0000000..7cf2214 --- /dev/null +++ b/tests/test_events/test_event.py @@ -0,0 +1,36 @@ +from lbz.events.event import Event + + +class MyTestEvent(Event): + type = "MY_TEST_EVENT" + + +class TestEvent: + def test_base_event_creation_and_structure(self) -> None: + event = {"x": 1} + new_event = MyTestEvent(event) + + assert new_event.type == "MY_TEST_EVENT" + assert new_event.data == {"x": 1} + assert new_event.serialized_data == '{"x": 1}' + + def test__eq__same(self) -> None: + new_event_1 = MyTestEvent({"x": 1}) + new_event_2 = MyTestEvent({"x": 1}) + + assert new_event_1 == new_event_2 + + def test__eq__different_data(self) -> None: + new_event_1 = MyTestEvent({"x": 1}) + new_event_2 = MyTestEvent({"x": 2}) + + assert new_event_1 != new_event_2 + + def test__eq__different_type_same_data(self) -> None: + class MySecondTestEvent(Event): + type = "MY_SECOND_TEST_EVENT" + + new_event_1 = MyTestEvent({"x": 1}) + new_event_2 = MySecondTestEvent({"x": 1}) + + assert new_event_1 != new_event_2 diff --git a/version b/version index 69626fb..9d6c175 100644 --- a/version +++ b/version @@ -1 +1 @@ -0.5.11 +0.5.12 From abeea2f214ed3b954023c75d44a5e01853e737b0 Mon Sep 17 00:00:00 2001 From: Piotr Dyba Date: Thu, 4 Aug 2022 13:00:35 +0200 Subject: [PATCH 2/4] Extends change log and adds a one more test --- CHANGELOG.md | 3 ++- tests/test_events/test_broker.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cb91a..494787d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -268,6 +268,7 @@ Released 2022-08-01 Released 2022-08-05 - Use the same class representing Event (both sent/received) +- Extends Event Broker with additional, optional parameters - Adds additional verbose test running to makefile -- Updates dev dependancies +- Updates dev dependencies diff --git a/tests/test_events/test_broker.py b/tests/test_events/test_broker.py index 838e5cf..07de8f6 100644 --- a/tests/test_events/test_broker.py +++ b/tests/test_events/test_broker.py @@ -21,6 +21,21 @@ def test_broker_works_properly(self) -> None: func_1.assert_called_once_with(expected_event) func_2.assert_called_once_with(expected_event) + def test_broker_works_properly_when_using_type_and_data_optional_keys(self) -> None: + func_1 = MagicMock() + expected_event = Event({"y": 1}, event_type="x") + mapper = {"x": [func_1]} + event = {"my-type-key": "x", "my-data-key": {"y": 1}} + + EventBroker( + mapper, # type: ignore + event, + type_key="my-type-key", + data_key="my-data-key", + ).handle() + + func_1.assert_called_once_with(expected_event) + def test_broker_raises_not_implemented_when_event_type_is_not_recognized(self) -> None: func_1 = MagicMock() func_2 = MagicMock() From e83e4f5f9546dbc3c33f00f0c9bb1d1f6ebc1faf Mon Sep 17 00:00:00 2001 From: Piotr Dyba Date: Fri, 5 Aug 2022 08:18:04 +0200 Subject: [PATCH 3/4] Changes from CodeReview --- lbz/events/broker.py | 1 + lbz/events/event.py | 4 ++-- setup.cfg | 4 ++-- tests/test_events/test_event.py | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lbz/events/broker.py b/lbz/events/broker.py index dddf1fc..fedea40 100644 --- a/lbz/events/broker.py +++ b/lbz/events/broker.py @@ -11,6 +11,7 @@ def __init__( self, mapper: Dict[str, List[Callable]], raw_event: dict, + *, type_key: str = "detail-type", data_key: str = "detail", ) -> None: diff --git a/lbz/events/event.py b/lbz/events/event.py index 64dbcca..e21d370 100644 --- a/lbz/events/event.py +++ b/lbz/events/event.py @@ -18,8 +18,8 @@ def __repr__(self) -> str: return f"Event(type='{self.type}', data={self.data})" @staticmethod - def serialize(raw_data: dict) -> str: - return json.dumps(raw_data, default=str) + def serialize(data: dict) -> str: + return json.dumps(data, default=str) @property def serialized_data(self) -> str: diff --git a/setup.cfg b/setup.cfg index bdd25aa..7c18420 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,8 +24,8 @@ use_parentheses = True [mypy] # https://mypy.readthedocs.io/en/latest/running_mypy.html#mapping-file-paths-to-modules explicit_package_bases = True -;mypy_path = -; $MYPY_CONFIG_FILE_DIR, +mypy_path = + $MYPY_CONFIG_FILE_DIR, namespace_packages = True # https://mypy.readthedocs.io/en/latest/config_file.html diff --git a/tests/test_events/test_event.py b/tests/test_events/test_event.py index 7cf2214..24f8f1d 100644 --- a/tests/test_events/test_event.py +++ b/tests/test_events/test_event.py @@ -6,7 +6,7 @@ class MyTestEvent(Event): class TestEvent: - def test_base_event_creation_and_structure(self) -> None: + def test_event_creation_and_structure(self) -> None: event = {"x": 1} new_event = MyTestEvent(event) From 377278f240a4f7c13da0b75763f75bdc992c0c6c Mon Sep 17 00:00:00 2001 From: Piotr Dyba Date: Fri, 5 Aug 2022 08:54:24 +0200 Subject: [PATCH 4/4] Changes from followup discussion --- lbz/events/api.py | 10 ---------- setup.cfg | 2 -- tests/test_events/test_api.py | 28 ++++++++++++++-------------- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/lbz/events/api.py b/lbz/events/api.py index 56a9d97..a99346e 100644 --- a/lbz/events/api.py +++ b/lbz/events/api.py @@ -57,16 +57,6 @@ def failed_events(self) -> List[Event]: def register(self, new_event: Event) -> None: self._pending_events.append(new_event) - # TODO: Stop sharing protected lists outside the class, use the above properties instead - def get_all_pending_events(self) -> List[Event]: - return self._pending_events - - def get_all_sent_events(self) -> List[Event]: - return self._sent_events - - def get_all_failed_events(self) -> List[Event]: - return self._failed_events - def send(self) -> None: self._sent_events = [] self._failed_events = [] diff --git a/setup.cfg b/setup.cfg index 7c18420..fbb021c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,8 +24,6 @@ use_parentheses = True [mypy] # https://mypy.readthedocs.io/en/latest/running_mypy.html#mapping-file-paths-to-modules explicit_package_bases = True -mypy_path = - $MYPY_CONFIG_FILE_DIR, namespace_packages = True # https://mypy.readthedocs.io/en/latest/config_file.html diff --git a/tests/test_events/test_api.py b/tests/test_events/test_api.py index f404d1c..7c22adc 100644 --- a/tests/test_events/test_api.py +++ b/tests/test_events/test_api.py @@ -79,7 +79,7 @@ def test__failed_events__disallows_changing_its_content_outside_api(self) -> Non assert self.event_api.failed_events == [] def test_register_saves_event_in_right_place(self) -> None: - assert self.event_api.get_all_pending_events() == [] + assert self.event_api.pending_events == [] event_1 = MyTestEvent({"x": 1}) event_2 = MyTestEvent({"x": 2}) @@ -87,7 +87,7 @@ def test_register_saves_event_in_right_place(self) -> None: self.event_api.register(event_1) self.event_api.register(event_2) - assert self.event_api.get_all_pending_events() == [event_1, event_2] + assert self.event_api.pending_events == [event_1, event_2] @patch.object(Boto3Client, "eventbridge") def test_send(self, mock_send: MagicMock) -> None: @@ -107,7 +107,7 @@ def test_send(self, mock_send: MagicMock) -> None: } ] ) - assert self.event_api.get_all_sent_events() == [event] + assert self.event_api.sent_events == [event] @patch.object(Boto3Client, "eventbridge") def test__send__sends_events_in_chunks_respecting_limits(self, mock_send: MagicMock) -> None: @@ -153,7 +153,7 @@ def test__send__always_tries_to_send_all_events_treating_each_chunk_individually @patch.object(Boto3Client, "eventbridge") def test_sent_fail_saves_events_in_right_place(self, mock_send: MagicMock) -> None: - assert self.event_api.get_all_failed_events() == [] + assert self.event_api.failed_events == [] mock_send.put_events.side_effect = NotADirectoryError event = MyTestEvent({"x": 1}) @@ -162,16 +162,16 @@ def test_sent_fail_saves_events_in_right_place(self, mock_send: MagicMock) -> No with pytest.raises(RuntimeError): self.event_api.send() - assert self.event_api.get_all_failed_events() == [event] + assert self.event_api.failed_events == [event] @patch.object(Boto3Client, "eventbridge") def test_send_no_events(self, mock_send: MagicMock) -> None: self.event_api.send() mock_send.put_events.assert_not_called() - assert self.event_api.get_all_failed_events() == [] - assert self.event_api.get_all_sent_events() == [] - assert self.event_api.get_all_pending_events() == [] + assert self.event_api.failed_events == [] + assert self.event_api.sent_events == [] + assert self.event_api.pending_events == [] @patch.object(Boto3Client, "eventbridge") def test_singleton_pattern_working_correctly_for_event_api(self, mock_send: MagicMock) -> None: @@ -206,9 +206,9 @@ def test_second_send_clears_everything(self) -> None: self.event_api.send() self.event_api.send() - assert self.event_api.get_all_failed_events() == [] - assert self.event_api.get_all_sent_events() == [] - assert self.event_api.get_all_pending_events() == [] + assert self.event_api.failed_events == [] + assert self.event_api.sent_events == [] + assert self.event_api.pending_events == [] @patch.object(Boto3Client, "eventbridge", MagicMock()) def test_clear(self) -> None: @@ -219,9 +219,9 @@ def test_clear(self) -> None: self.event_api.clear() - assert self.event_api.get_all_failed_events() == [] - assert self.event_api.get_all_sent_events() == [] - assert self.event_api.get_all_pending_events() == [] + assert self.event_api.failed_events == [] + assert self.event_api.sent_events == [] + assert self.event_api.pending_events == [] @patch.object(Boto3Client, "eventbridge", MagicMock())