From 726f5eb0c1ca3a2127a8604beb10e2c60100dd43 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Tue, 26 Sep 2023 15:38:56 -0400 Subject: [PATCH 01/21] feat!: new event bus config format --- openedx_events/apps.py | 49 ++++++++++++++------ openedx_events/tests/test_producer_config.py | 19 ++++---- test_utils/test_settings.py | 29 ++++-------- 3 files changed, 53 insertions(+), 44 deletions(-) diff --git a/openedx_events/apps.py b/openedx_events/apps.py index 0219eb95..95b2bdd5 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -14,14 +14,15 @@ def general_signal_handler(sender, signal, **kwargs): # pylint: disable=unused- """ Signal handler for publishing events to configured event bus. """ - configurations = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, ()) + configurations = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) event_data = {key: kwargs.get(key) for key in signal.init_data} - for configuration in configurations: - if configuration["enabled"]: + + for topic in configurations.keys(): + if configurations[topic]["enabled"]: get_producer().send( signal=signal, - topic=configuration["topic"], - event_key_field=configuration["event_key_field"], + topic=topic, + event_key_field=configurations[topic]["event_key_field"], event_data=event_data, event_metadata=kwargs["metadata"], ) @@ -34,40 +35,47 @@ class OpenedxEventsConfig(AppConfig): name = "openedx_events" - def _get_validated_signal_config(self, event_type, configurations): + def _get_validated_signal_config(self, event_type, configuration): """ Validate signal configuration format. + Example expected signal configuration: + { + "topic_a": { "event_key_field": "my.key.field", "enabled": True }, + "topic_b": { "event_key_field": "my.key.field", "enabled": False } + } + Raises: ProducerConfigurationError: If configuration is not valid. """ - if not isinstance(configurations, list) and not isinstance(configurations, tuple): + if not isinstance(configuration, dict) and not isinstance(configuration, dict): raise ProducerConfigurationError( event_type=event_type, - message="Configuration for event_types should be a list or a tuple of dictionaries" + message="Configuration for event_types should be a dict" ) try: signal = OpenEdxPublicSignal.get_signal_by_type(event_type) except KeyError as exc: raise ProducerConfigurationError(message=f"No OpenEdxPublicSignal of type: '{event_type}'.") from exc - for configuration in configurations: - if not isinstance(configuration, dict): + for topic, topic_configuration in configuration.items(): + print(f"{topic_configuration=}") + if not isinstance(topic_configuration, dict): raise ProducerConfigurationError( event_type=event_type, - message="One of the configuration object is not a dictionary" + message="One of the configuration objects is not a dictionary" ) - expected_keys = {"topic": str, "event_key_field": str, "enabled": bool} + expected_keys = {"event_key_field": str, "enabled": bool} for expected_key, expected_type in expected_keys.items(): - if expected_key not in configuration: + if expected_key not in topic_configuration.keys(): raise ProducerConfigurationError( event_type=event_type, message=f"One of the configuration object is missing '{expected_key}' key." ) - if not isinstance(configuration[expected_key], expected_type): + if not isinstance(topic_configuration[expected_key], expected_type): raise ProducerConfigurationError( event_type=event_type, message=(f"Expected type: {expected_type} for '{expected_key}', " - f"found: {type(configuration[expected_key])}") + f"found: {type(topic_configuration[expected_key])}") ) return signal @@ -75,6 +83,17 @@ def ready(self): """ Read `EVENT_BUS_PRODUCER_CONFIG` setting and connects appropriate handlers to the events based on it. + Example expected configuration: + { + "org.openedx.content_authoring.xblock.deleted.v1" : { + "topic_a": { "event_key_field": "xblock_info.usage_key", "enabled": True }, + "topic_b": { "event_key_field": "xblock_info.usage_key", "enabled": False } + }, + "org.openedx.content_authoring.course.catalog_info.changed.v1" : { + "topic_c": {"event_key_field": "course_info.course_key", "enabled": True } + } + } + Raises: ProducerConfigurationError: If `EVENT_BUS_PRODUCER_CONFIG` is not valid. """ diff --git a/openedx_events/tests/test_producer_config.py b/openedx_events/tests/test_producer_config.py index ee84bd79..b558c856 100644 --- a/openedx_events/tests/test_producer_config.py +++ b/openedx_events/tests/test_producer_config.py @@ -78,21 +78,23 @@ def test_configuration_is_validated(self): with pytest.raises(ProducerConfigurationError, match="should be a dictionary"): apps.get_app_config("openedx_events").ready() - with override_settings(EVENT_BUS_PRODUCER_CONFIG={"invalid.event.type": []}): + with override_settings(EVENT_BUS_PRODUCER_CONFIG={"invalid.event.type": {}}): with pytest.raises(ProducerConfigurationError, match="No OpenEdxPublicSignal of type"): apps.get_app_config("openedx_events").ready() with override_settings(EVENT_BUS_PRODUCER_CONFIG={"org.openedx.content_authoring.xblock.deleted.v1": ""}): - with pytest.raises(ProducerConfigurationError, match="should be a list or a tuple"): + with pytest.raises(ProducerConfigurationError, match="should be a dict"): apps.get_app_config("openedx_events").ready() - with override_settings(EVENT_BUS_PRODUCER_CONFIG={"org.openedx.content_authoring.xblock.deleted.v1": [""]}): - with pytest.raises(ProducerConfigurationError, match="object is not a dictionary"): + with override_settings(EVENT_BUS_PRODUCER_CONFIG={"org.openedx.content_authoring.xblock.deleted.v1": + {"topic": ""}}): + with pytest.raises(ProducerConfigurationError, match="One of the configuration objects is not a" + " dictionary"): apps.get_app_config("openedx_events").ready() with override_settings( EVENT_BUS_PRODUCER_CONFIG={ - "org.openedx.content_authoring.xblock.deleted.v1": [{"topic": "some", "enabled": True}] + "org.openedx.content_authoring.xblock.deleted.v1": {"some": {"enabled": True}} } ): with pytest.raises(ProducerConfigurationError, match="missing 'event_key_field' key."): @@ -100,9 +102,10 @@ def test_configuration_is_validated(self): with override_settings( EVENT_BUS_PRODUCER_CONFIG={ - "org.openedx.content_authoring.xblock.deleted.v1": [ - {"topic": "some", "enabled": 1, "event_key_field": "some"} - ] + "org.openedx.content_authoring.xblock.deleted.v1": + { + "some": {"enabled": 1, "event_key_field": "some"} + } } ): with pytest.raises( diff --git a/test_utils/test_settings.py b/test_utils/test_settings.py index a0fd9143..096aa155 100644 --- a/test_utils/test_settings.py +++ b/test_utils/test_settings.py @@ -30,28 +30,15 @@ SECRET_KEY = "not-so-secret-key" EVENT_BUS_PRODUCER_CONFIG = { - 'org.openedx.content_authoring.xblock.published.v1': ( + 'org.openedx.content_authoring.xblock.published.v1': { - 'topic': 'content-authoring-xblock-lifecycle', - 'event_key_field': 'xblock_info.usage_key', - 'enabled': True + 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + 'content-authoring-all-status': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + 'content-authoring-xblock-published': {'event_key_field': 'xblock_info.usage_key', 'enabled': False} }, + 'org.openedx.content_authoring.xblock.deleted.v1': { - 'topic': 'content-authoring-all-status', - 'event_key_field': 'xblock_info.usage_key', - 'enabled': True - }, - { - 'topic': 'content-authoring-xblock-published', - 'event_key_field': 'xblock_info.usage_key', - 'enabled': False - }, - ), - 'org.openedx.content_authoring.xblock.deleted.v1': ( - { - 'topic': 'content-authoring-xblock-lifecycle', - 'event_key_field': 'xblock_info.usage_key', - 'enabled': True - }, - ), + 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + } } + From 688a72f4687e16bfa9abd7868710ad6983aaedc0 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 28 Sep 2023 16:41:31 -0400 Subject: [PATCH 02/21] fixup!: ADR --- .../0013-new-event-bus-producer-config.rst | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 docs/decisions/0013-new-event-bus-producer-config.rst diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst new file mode 100644 index 00000000..21e19047 --- /dev/null +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -0,0 +1,65 @@ +11. Enable producing to event bus via settings +############################################## + +Status +****** + +**Accepted** + +Context +******* + +In a previous ADR, we set the structure for the event bus publishing configuration to a dictionary like the following: + +.. code-block:: python + + { 'org.openedx.content_authoring.xblock.published.v1': [ + {'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + {'topic': 'content-authoring-xblock-published', 'event_key_field': 'xblock_info.usage_key', 'enabled': False}, + ], + 'org.openedx.content_authoring.xblock.deleted.v1': [ + {'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + ], + } + +While attempting to implement this for edx-platform, we came across some problems with using this structure. In particular, it results in ambiguity +because maintainers can accidentally add something like +``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': True}`` and +``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': False}`` to the same event_type. +Also, as written, this configuration requires maintainers to copy over the entire configuration to override it, which is non-obvious +to people who may only be trying to enable/disable a single event while keeping everything else the same. + +This ADR aims to propose a new structure that will provide greater flexibility in using this configuration. + + +Decision +******** + +The new EVENT_BUS_PRODUCER_CONFIG will be added to KEYS_WITH_MERGED_VALUES in ``edx-platform`` with the following configuration format: + +.. code-block:: python + + # .. setting_name: EVENT_BUS_PRODUCER_CONFIG + # .. setting_default: {} + # .. setting_description: Dictionary of event_types to dictionaries for topic related configuration. + # Each topic configuration dictionary uses the topic as a key and contains a flag called `enabled` + # denoting whether the event will be and `event_key_field` which is a period-delimited string path + # to event data field to use as event key. + # Note: The topic names should not include environment prefix as it will be dynamically added based on + # EVENT_BUS_TOPIC_PREFIX setting. + EVENT_BUS_PRODUCER_CONFIG = { + 'org.openedx.content_authoring.xblock.published.v1': { + 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': False} + 'content-authoring-xblock-published': {'event_key_field': 'xblock_info.usage_key', 'enabled': True} + }, + 'org.openedx.content_authoring.xblock.deleted.v1': { + 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + }, + } + + +Consequences +************ + +* Maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* There is no ambiguity about whether an events is enabled or disabled From 28f56626916c0dbfae5ea20159ed2e61367bed0b Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 28 Sep 2023 16:56:02 -0400 Subject: [PATCH 03/21] fixup!: ADR --- .../decisions/0013-new-event-bus-producer-config.rst | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst index 21e19047..fb3306aa 100644 --- a/docs/decisions/0013-new-event-bus-producer-config.rst +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -27,7 +27,9 @@ because maintainers can accidentally add something like ``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': True}`` and ``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': False}`` to the same event_type. Also, as written, this configuration requires maintainers to copy over the entire configuration to override it, which is non-obvious -to people who may only be trying to enable/disable a single event while keeping everything else the same. +to people who may only be trying to enable/disable a single event while keeping everything else the same. Moreover, it's also non-obvious +that enabling/disabling an existing event/topic pair requires reaching into the structure, searching for the dictionary with the correct topic, and modifying +the existing object, which is awkward. This ADR aims to propose a new structure that will provide greater flexibility in using this configuration. @@ -35,7 +37,7 @@ This ADR aims to propose a new structure that will provide greater flexibility i Decision ******** -The new EVENT_BUS_PRODUCER_CONFIG will be added to KEYS_WITH_MERGED_VALUES in ``edx-platform`` with the following configuration format: +The new EVENT_BUS_PRODUCER_CONFIG will have the following configuration format: .. code-block:: python @@ -57,9 +59,11 @@ The new EVENT_BUS_PRODUCER_CONFIG will be added to KEYS_WITH_MERGED_VALUES in `` }, } +In edx-platform, it will be added to the KEYS_WITH_MERGED_VALUES list to allow partial additions/overrides. Consequences ************ -* Maintainers can add existing topics to new event_types without having to recreate the whole dictionary -* There is no ambiguity about whether an events is enabled or disabled +* As long as the implementing IDA has a concept of KEYS_WITH_MERGED_VALUES (more complex configurations that can be modified via code in settings), +maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* There is no ambiguity about whether an event/topic pair is enabled or disabled From ad615f08b5f68ef199ac50e5c8ac7dd4b6e74484 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 28 Sep 2023 16:56:59 -0400 Subject: [PATCH 04/21] fixup!: ADR --- docs/decisions/0013-new-event-bus-producer-config.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst index fb3306aa..104541df 100644 --- a/docs/decisions/0013-new-event-bus-producer-config.rst +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -1,5 +1,5 @@ -11. Enable producing to event bus via settings -############################################## +13. Change event producer config settings +######################################### Status ****** From d62ca0569899e4eeb60056016e186288bada4ea0 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Fri, 29 Sep 2023 08:12:43 -0400 Subject: [PATCH 05/21] fixup!: docs --- docs/decisions/0013-new-event-bus-producer-config.rst | 4 +--- docs/decisions/index.rst | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst index 104541df..b0381fbe 100644 --- a/docs/decisions/0013-new-event-bus-producer-config.rst +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -33,7 +33,6 @@ the existing object, which is awkward. This ADR aims to propose a new structure that will provide greater flexibility in using this configuration. - Decision ******** @@ -64,6 +63,5 @@ In edx-platform, it will be added to the KEYS_WITH_MERGED_VALUES list to allow p Consequences ************ -* As long as the implementing IDA has a concept of KEYS_WITH_MERGED_VALUES (more complex configurations that can be modified via code in settings), -maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* As long as the implementing IDA has a concept of KEYS_WITH_MERGED_VALUES (more complex configurations that can be modified via code in settings), maintainers can add existing topics to new event_types without having to recreate the whole dictionary * There is no ambiguity about whether an event/topic pair is enabled or disabled diff --git a/docs/decisions/index.rst b/docs/decisions/index.rst index 81479709..95fe4597 100644 --- a/docs/decisions/index.rst +++ b/docs/decisions/index.rst @@ -17,3 +17,4 @@ Architectural Decision Records (ADRs) 0010-multiple-event-types-per-topic 0011-depending-on-multiple-event-bus-implementations 0012-producing-to-event-bus-via-settings + 0013-new-event-bus-producer-config From bc0097c9c4f51bb732d104e3a832f36dcd39fd57 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Fri, 29 Sep 2023 09:25:14 -0400 Subject: [PATCH 06/21] fixup!: quality --- openedx_events/apps.py | 2 +- test_utils/test_settings.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/openedx_events/apps.py b/openedx_events/apps.py index 95b2bdd5..42ff05ba 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -57,7 +57,7 @@ def _get_validated_signal_config(self, event_type, configuration): signal = OpenEdxPublicSignal.get_signal_by_type(event_type) except KeyError as exc: raise ProducerConfigurationError(message=f"No OpenEdxPublicSignal of type: '{event_type}'.") from exc - for topic, topic_configuration in configuration.items(): + for _, topic_configuration in configuration.items(): print(f"{topic_configuration=}") if not isinstance(topic_configuration, dict): raise ProducerConfigurationError( diff --git a/test_utils/test_settings.py b/test_utils/test_settings.py index 096aa155..d4edc940 100644 --- a/test_utils/test_settings.py +++ b/test_utils/test_settings.py @@ -41,4 +41,3 @@ 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, } } - From 90f844026687d3c62f5d4553cc503542f8613b1a Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 08:31:05 -0400 Subject: [PATCH 07/21] fixup!: merge configs --- .../0013-new-event-bus-producer-config.rst | 10 ++--- openedx_events/event_bus/__init__.py | 13 ++++++ openedx_events/event_bus/tests/test_loader.py | 40 ++++++++++++++++++- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst index b0381fbe..5eeb817a 100644 --- a/docs/decisions/0013-new-event-bus-producer-config.rst +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -26,9 +26,7 @@ While attempting to implement this for edx-platform, we came across some problem because maintainers can accidentally add something like ``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': True}`` and ``{'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key', 'enabled': False}`` to the same event_type. -Also, as written, this configuration requires maintainers to copy over the entire configuration to override it, which is non-obvious -to people who may only be trying to enable/disable a single event while keeping everything else the same. Moreover, it's also non-obvious -that enabling/disabling an existing event/topic pair requires reaching into the structure, searching for the dictionary with the correct topic, and modifying +Moreover, enabling/disabling an existing event/topic pair requires reaching into the structure, searching for the dictionary with the correct topic, and modifying the existing object, which is awkward. This ADR aims to propose a new structure that will provide greater flexibility in using this configuration. @@ -50,7 +48,7 @@ The new EVENT_BUS_PRODUCER_CONFIG will have the following configuration format: # EVENT_BUS_TOPIC_PREFIX setting. EVENT_BUS_PRODUCER_CONFIG = { 'org.openedx.content_authoring.xblock.published.v1': { - 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': False} + 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': False}, 'content-authoring-xblock-published': {'event_key_field': 'xblock_info.usage_key', 'enabled': True} }, 'org.openedx.content_authoring.xblock.deleted.v1': { @@ -58,10 +56,10 @@ The new EVENT_BUS_PRODUCER_CONFIG will have the following configuration format: }, } -In edx-platform, it will be added to the KEYS_WITH_MERGED_VALUES list to allow partial additions/overrides. +A new ``merge_configs`` method will be added to openedx-events.event_bus to make it easier to correctly determine the config map from multiple sources. Consequences ************ -* As long as the implementing IDA has a concept of KEYS_WITH_MERGED_VALUES (more complex configurations that can be modified via code in settings), maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* As long as the implementing IDA calls ``merge_configs``, maintainers can add existing topics to new event_types without having to recreate the whole dictionary * There is no ambiguity about whether an event/topic pair is enabled or disabled diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index 582effed..b846d23f 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -183,3 +183,16 @@ def make_single_consumer(*, topic: str, group_id: str, def _reset_state(sender, **kwargs): # pylint: disable=unused-argument """Reset caches when settings change during unit tests.""" get_producer.cache_clear() + + +def merge_publisher_configs(publisher_config_a, publisher_config_b): + combined = {**publisher_config_a} + for event_type, event_type_config_b in publisher_config_b.items(): + event_type_config_combined = combined.get(event_type, {}) + for topic, topic_config_b in event_type_config_b.items(): + topic_config_combined = event_type_config_combined.get(topic, {}) + topic_config_combined['enabled'] = topic_config_b['enabled'] + topic_config_combined['event_key_field'] = topic_config_b['event_key_field'] + event_type_config_combined[topic] = topic_config_combined + combined[event_type] = event_type_config_combined + return combined diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index 07934bf1..2ce2a874 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -9,7 +9,7 @@ from django.test import override_settings from openedx_events.data import EventsMetadata -from openedx_events.event_bus import _try_load, get_producer, make_single_consumer +from openedx_events.event_bus import _try_load, get_producer, make_single_consumer, merge_publisher_configs from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED @@ -126,3 +126,41 @@ def test_default_does_nothing(self): with assert_warnings([]): # Nothing thrown, no warnings. assert consumer.consume_indefinitely() is None + +class TestSettings(TestCase): + def test_merge_configs(self): + dict_a = { + 'event_type_0': { + 'topic_a': {'event_key_field': 'field', 'enabled': True}, + 'topic_b': {'event_key_field': 'field', 'enabled': True} + }, + 'event_type_1': { + 'topic_c': {'event_key_field': 'field', 'enabled': True}, + } + } + dict_b = { + 'event_type_0': { + # disable an existing event/topic pairing + 'topic_a': {'event_key_field': 'field', 'enabled': False}, + # add a new topic to an existing topic + 'topic_d': {'event_key_field': 'field', 'enabled': True}, + }, + # add a new event_type + 'event_type_2': { + 'topic_e': {'event_key_field': 'field', 'enabled': True}, + } + } + result = merge_publisher_configs(dict_a, dict_b) + self.assertDictEqual(result, { + 'event_type_0': { + 'topic_a': {'event_key_field': 'field', 'enabled': False}, + 'topic_b': {'event_key_field': 'field', 'enabled': True}, + 'topic_d': {'event_key_field': 'field', 'enabled': True}, + }, + 'event_type_1': { + 'topic_c': {'event_key_field': 'field', 'enabled': True}, + }, + 'event_type_2': { + 'topic_e': {'event_key_field': 'field', 'enabled': True}, + } + }) From 4340f986c0d53cca0259b3a707d59e4ee58aa7f0 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 08:31:40 -0400 Subject: [PATCH 08/21] fixup!: merge configs --- docs/decisions/0013-new-event-bus-producer-config.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0013-new-event-bus-producer-config.rst index 5eeb817a..0ba24b9f 100644 --- a/docs/decisions/0013-new-event-bus-producer-config.rst +++ b/docs/decisions/0013-new-event-bus-producer-config.rst @@ -56,10 +56,10 @@ The new EVENT_BUS_PRODUCER_CONFIG will have the following configuration format: }, } -A new ``merge_configs`` method will be added to openedx-events.event_bus to make it easier to correctly determine the config map from multiple sources. +A new ``merge_publisher_configs`` method will be added to openedx-events.event_bus to make it easier to correctly determine the config map from multiple sources. Consequences ************ -* As long as the implementing IDA calls ``merge_configs``, maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* As long as the implementing IDA calls ``merge_publisher_configs``, maintainers can add existing topics to new event_types without having to recreate the whole dictionary * There is no ambiguity about whether an event/topic pair is enabled or disabled From 4c9578ec59fd81b6f618676995444b55f5ab355a Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 13:21:11 -0400 Subject: [PATCH 09/21] fixup!: versioning --- CHANGELOG.rst | 6 ++++-- openedx_events/__init__.py | 2 +- openedx_events/event_bus/tests/test_loader.py | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b833062d..bb9685d8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,9 +13,11 @@ Change Log Unreleased ---------- -Changed -~~~~~~~ + +[9.0.0] - 2023-10-04 +-------------------- * Re-licensed this repository from AGPL 3.0 to Apache 2.0 +* **Breaking change**: Restructured EVENT_BUS_PRODUCER_CONFIG [8.6.0] - 2023-08-28 -------------------- diff --git a/openedx_events/__init__.py b/openedx_events/__init__.py index 3feb7fdf..977cc1ca 100644 --- a/openedx_events/__init__.py +++ b/openedx_events/__init__.py @@ -5,4 +5,4 @@ more information about the project. """ -__version__ = "8.6.0" +__version__ = "9.0.0" diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index 2ce2a874..7d279df4 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -164,3 +164,17 @@ def test_merge_configs(self): 'topic_e': {'event_key_field': 'field', 'enabled': True}, } }) + + def test_merge_configs_with_empty(self): + dict_a = { + 'event_type_0': { + 'topic_a': {'event_key_field': 'field', 'enabled': True}, + 'topic_b': {'event_key_field': 'field', 'enabled': True} + }, + 'event_type_1': { + 'topic_c': {'event_key_field': 'field', 'enabled': True}, + } + } + dict_b = {} + result = merge_publisher_configs(dict_a, dict_b) + self.assertDictEqual(result, dict_a) From 815bdf5a73ee4317d829fc66e131d8715b18702d Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 13:25:38 -0400 Subject: [PATCH 10/21] fixup!: renumber --- ...oducer-config.rst => 0014-new-event-bus-producer-config.rst} | 0 docs/decisions/index.rst | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/decisions/{0013-new-event-bus-producer-config.rst => 0014-new-event-bus-producer-config.rst} (100%) diff --git a/docs/decisions/0013-new-event-bus-producer-config.rst b/docs/decisions/0014-new-event-bus-producer-config.rst similarity index 100% rename from docs/decisions/0013-new-event-bus-producer-config.rst rename to docs/decisions/0014-new-event-bus-producer-config.rst diff --git a/docs/decisions/index.rst b/docs/decisions/index.rst index abb5c5d2..fbf7a44a 100644 --- a/docs/decisions/index.rst +++ b/docs/decisions/index.rst @@ -18,4 +18,4 @@ Architectural Decision Records (ADRs) 0011-depending-on-multiple-event-bus-implementations 0012-producing-to-event-bus-via-settings 0013-new-event-bus-producer-config - 0013-special-exam-submission-and-review-events + 0014-special-exam-submission-and-review-events From c80593137b0b08ae497cabf874370101c9654fc9 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 13:53:07 -0400 Subject: [PATCH 11/21] fixup!: docstring --- openedx_events/event_bus/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index b846d23f..99d6a2e7 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -186,6 +186,18 @@ def _reset_state(sender, **kwargs): # pylint: disable=unused-argument def merge_publisher_configs(publisher_config_a, publisher_config_b): + """ + Merge two EVENT_BUS_PRODUCER_CONFIG maps + + Arguments: + publisher_config_a: An EVENT_BUS_PRODUCER_CONFIG-structured map + publisher_config_b: An EVENT_BUS_PRODUCER_CONFIG-structured map + + Returns: + A new EVENT_BUS_PRODUCER_CONFIG map created by combining the two maps. All event_type/topic pairs in + publisher_config_b are added to the publisher_config_a. If there is a conflict on whether a particular + event_type/topic pair is enabled, publisher_config_b wins out. + """ combined = {**publisher_config_a} for event_type, event_type_config_b in publisher_config_b.items(): event_type_config_combined = combined.get(event_type, {}) From f01b55d35997f35a2aa6819202ee18227d779861 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 14:04:42 -0400 Subject: [PATCH 12/21] fixup!: fix index --- docs/decisions/index.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/decisions/index.rst b/docs/decisions/index.rst index fbf7a44a..f059dd83 100644 --- a/docs/decisions/index.rst +++ b/docs/decisions/index.rst @@ -17,5 +17,6 @@ Architectural Decision Records (ADRs) 0010-multiple-event-types-per-topic 0011-depending-on-multiple-event-bus-implementations 0012-producing-to-event-bus-via-settings - 0013-new-event-bus-producer-config - 0014-special-exam-submission-and-review-events + 0013-special-exam-submission-and-review-events + 0014-new-event-bus-producer-config + From 034d4d69a877cf7c0f02524b9c5d522f756df5d4 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 14:32:26 -0400 Subject: [PATCH 13/21] fixup!: quality --- openedx_events/apps.py | 2 +- openedx_events/event_bus/__init__.py | 2 +- openedx_events/event_bus/tests/test_loader.py | 1 + openedx_events/tests/test_producer_config.py | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openedx_events/apps.py b/openedx_events/apps.py index 42ff05ba..de5e4748 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -75,7 +75,7 @@ def _get_validated_signal_config(self, event_type, configuration): raise ProducerConfigurationError( event_type=event_type, message=(f"Expected type: {expected_type} for '{expected_key}', " - f"found: {type(topic_configuration[expected_key])}") + f"found: {type(topic_configuration[expected_key])}") ) return signal diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index 99d6a2e7..80a0f2cb 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -187,7 +187,7 @@ def _reset_state(sender, **kwargs): # pylint: disable=unused-argument def merge_publisher_configs(publisher_config_a, publisher_config_b): """ - Merge two EVENT_BUS_PRODUCER_CONFIG maps + Merge two EVENT_BUS_PRODUCER_CONFIG maps. Arguments: publisher_config_a: An EVENT_BUS_PRODUCER_CONFIG-structured map diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index 7d279df4..f4c4e208 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -127,6 +127,7 @@ def test_default_does_nothing(self): # Nothing thrown, no warnings. assert consumer.consume_indefinitely() is None + class TestSettings(TestCase): def test_merge_configs(self): dict_a = { diff --git a/openedx_events/tests/test_producer_config.py b/openedx_events/tests/test_producer_config.py index b558c856..16ccecd2 100644 --- a/openedx_events/tests/test_producer_config.py +++ b/openedx_events/tests/test_producer_config.py @@ -87,7 +87,7 @@ def test_configuration_is_validated(self): apps.get_app_config("openedx_events").ready() with override_settings(EVENT_BUS_PRODUCER_CONFIG={"org.openedx.content_authoring.xblock.deleted.v1": - {"topic": ""}}): + {"topic": ""}}): with pytest.raises(ProducerConfigurationError, match="One of the configuration objects is not a" " dictionary"): apps.get_app_config("openedx_events").ready() From e36a22b23f14bc1ef7834493849bcf8ff4a195c3 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 4 Oct 2023 14:37:57 -0400 Subject: [PATCH 14/21] fixup!: fix test? --- openedx_events/event_bus/tests/test_loader.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index f4c4e208..fee2a6c6 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -166,16 +166,16 @@ def test_merge_configs(self): } }) - def test_merge_configs_with_empty(self): - dict_a = { - 'event_type_0': { - 'topic_a': {'event_key_field': 'field', 'enabled': True}, - 'topic_b': {'event_key_field': 'field', 'enabled': True} - }, - 'event_type_1': { - 'topic_c': {'event_key_field': 'field', 'enabled': True}, - } + def test_merge_configs_with_empty(self): + dict_a = { + 'event_type_0': { + 'topic_a': {'event_key_field': 'field', 'enabled': True}, + 'topic_b': {'event_key_field': 'field', 'enabled': True} + }, + 'event_type_1': { + 'topic_c': {'event_key_field': 'field', 'enabled': True}, } - dict_b = {} - result = merge_publisher_configs(dict_a, dict_b) - self.assertDictEqual(result, dict_a) + } + dict_b = {} + result = merge_publisher_configs(dict_a, dict_b) + self.assertDictEqual(result, dict_a) From c82fc3b6e54af6329f87966fdfb1671e039297ea Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 08:34:07 -0400 Subject: [PATCH 15/21] fixup!: changes from review --- CHANGELOG.rst | 2 ++ openedx_events/apps.py | 14 +++++++++----- openedx_events/event_bus/__init__.py | 11 ++++++----- openedx_events/event_bus/tests/test_loader.py | 7 +++++++ openedx_events/tests/test_producer_config.py | 6 +++--- test_utils/test_settings.py | 6 +++--- 6 files changed, 30 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8cd38d57..68cb1238 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,8 @@ Unreleased [9.0.0] - 2023-10-04 -------------------- +Changed +~~~~~~~ * Re-licensed this repository from AGPL 3.0 to Apache 2.0 * **Breaking change**: Restructured EVENT_BUS_PRODUCER_CONFIG diff --git a/openedx_events/apps.py b/openedx_events/apps.py index de5e4748..7221603a 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -14,15 +14,20 @@ def general_signal_handler(sender, signal, **kwargs): # pylint: disable=unused- """ Signal handler for publishing events to configured event bus. """ - configurations = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) + event_type_publish_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) + # event_type_publish_configs should look something like + # { + # "topic_a": { "event_key_field": "my.key.field", "enabled": True }, + # "topic_b": { "event_key_field": "my.key.field", "enabled": False } + # }" event_data = {key: kwargs.get(key) for key in signal.init_data} - for topic in configurations.keys(): - if configurations[topic]["enabled"]: + for topic in event_type_publish_configs.keys(): + if event_type_publish_configs[topic]["enabled"] is True: get_producer().send( signal=signal, topic=topic, - event_key_field=configurations[topic]["event_key_field"], + event_key_field=event_type_publish_configs[topic]["event_key_field"], event_data=event_data, event_metadata=kwargs["metadata"], ) @@ -58,7 +63,6 @@ def _get_validated_signal_config(self, event_type, configuration): except KeyError as exc: raise ProducerConfigurationError(message=f"No OpenEdxPublicSignal of type: '{event_type}'.") from exc for _, topic_configuration in configuration.items(): - print(f"{topic_configuration=}") if not isinstance(topic_configuration, dict): raise ProducerConfigurationError( event_type=event_type, diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index 80a0f2cb..eb3002ca 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -14,6 +14,7 @@ ``EVENT_BUS_CONSUMER``. """ +import copy import warnings from abc import ABC, abstractmethod from functools import lru_cache @@ -185,21 +186,21 @@ def _reset_state(sender, **kwargs): # pylint: disable=unused-argument get_producer.cache_clear() -def merge_publisher_configs(publisher_config_a, publisher_config_b): +def merge_publisher_configs(publisher_config_original, publisher_config_overrides): """ Merge two EVENT_BUS_PRODUCER_CONFIG maps. Arguments: - publisher_config_a: An EVENT_BUS_PRODUCER_CONFIG-structured map - publisher_config_b: An EVENT_BUS_PRODUCER_CONFIG-structured map + publisher_config_original: An EVENT_BUS_PRODUCER_CONFIG-structured map + publisher_config_overrides: An EVENT_BUS_PRODUCER_CONFIG-structured map Returns: A new EVENT_BUS_PRODUCER_CONFIG map created by combining the two maps. All event_type/topic pairs in publisher_config_b are added to the publisher_config_a. If there is a conflict on whether a particular event_type/topic pair is enabled, publisher_config_b wins out. """ - combined = {**publisher_config_a} - for event_type, event_type_config_b in publisher_config_b.items(): + combined = copy.deepcopy(publisher_config_original) + for event_type, event_type_config_b in publisher_config_overrides.items(): event_type_config_combined = combined.get(event_type, {}) for topic, topic_config_b in event_type_config_b.items(): topic_config_combined = event_type_config_combined.get(topic, {}) diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index fee2a6c6..b7d147aa 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -2,6 +2,7 @@ Tests for event bus implementation loader. """ +import copy import warnings from contextlib import contextmanager from unittest import TestCase @@ -139,6 +140,8 @@ def test_merge_configs(self): 'topic_c': {'event_key_field': 'field', 'enabled': True}, } } + # for ensuring we didn't change the original dict + dict_a_copy = copy.deepcopy(dict_a) dict_b = { 'event_type_0': { # disable an existing event/topic pairing @@ -151,6 +154,7 @@ def test_merge_configs(self): 'topic_e': {'event_key_field': 'field', 'enabled': True}, } } + dict_b_copy = copy.deepcopy(dict_b) result = merge_publisher_configs(dict_a, dict_b) self.assertDictEqual(result, { 'event_type_0': { @@ -165,6 +169,8 @@ def test_merge_configs(self): 'topic_e': {'event_key_field': 'field', 'enabled': True}, } }) + self.assertDictEqual(dict_a, dict_a_copy) + self.assertDictEqual(dict_b, dict_b_copy) def test_merge_configs_with_empty(self): dict_a = { @@ -179,3 +185,4 @@ def test_merge_configs_with_empty(self): dict_b = {} result = merge_publisher_configs(dict_a, dict_b) self.assertDictEqual(result, dict_a) + diff --git a/openedx_events/tests/test_producer_config.py b/openedx_events/tests/test_producer_config.py index 16ccecd2..61183827 100644 --- a/openedx_events/tests/test_producer_config.py +++ b/openedx_events/tests/test_producer_config.py @@ -46,12 +46,12 @@ def test_enabled_disabled_events(self, mock_producer): # check that call_args_list only consists of enabled topics. call_args = mock_send.send.call_args_list[0][1] self.assertDictContainsSubset( - {'topic': 'content-authoring-xblock-lifecycle', 'event_key_field': 'xblock_info.usage_key'}, + {'topic': 'enabled_topic_a', 'event_key_field': 'xblock_info.usage_key'}, call_args ) call_args = mock_send.send.call_args_list[1][1] self.assertDictContainsSubset( - {'topic': 'content-authoring-all-status', 'event_key_field': 'xblock_info.usage_key'}, + {'topic': 'enabled_topic_b', 'event_key_field': 'xblock_info.usage_key'}, call_args ) @@ -94,7 +94,7 @@ def test_configuration_is_validated(self): with override_settings( EVENT_BUS_PRODUCER_CONFIG={ - "org.openedx.content_authoring.xblock.deleted.v1": {"some": {"enabled": True}} + "org.openedx.content_authoring.xblock.deleted.v1": {"topic": {"enabled": True}} } ): with pytest.raises(ProducerConfigurationError, match="missing 'event_key_field' key."): diff --git a/test_utils/test_settings.py b/test_utils/test_settings.py index d4edc940..2359bfff 100644 --- a/test_utils/test_settings.py +++ b/test_utils/test_settings.py @@ -32,9 +32,9 @@ EVENT_BUS_PRODUCER_CONFIG = { 'org.openedx.content_authoring.xblock.published.v1': { - 'content-authoring-xblock-lifecycle': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, - 'content-authoring-all-status': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, - 'content-authoring-xblock-published': {'event_key_field': 'xblock_info.usage_key', 'enabled': False} + 'enabled_topic_a': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + 'enabled_topic_b': {'event_key_field': 'xblock_info.usage_key', 'enabled': True}, + 'disabled_topic': {'event_key_field': 'xblock_info.usage_key', 'enabled': False} }, 'org.openedx.content_authoring.xblock.deleted.v1': { From 1532ffb090f282e6f17e22571034d638f10e6193 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 08:35:15 -0400 Subject: [PATCH 16/21] fixup!: space --- docs/decisions/index.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/decisions/index.rst b/docs/decisions/index.rst index f059dd83..627c4c3d 100644 --- a/docs/decisions/index.rst +++ b/docs/decisions/index.rst @@ -19,4 +19,3 @@ Architectural Decision Records (ADRs) 0012-producing-to-event-bus-via-settings 0013-special-exam-submission-and-review-events 0014-new-event-bus-producer-config - From eb2d9acd409bb1030506405cf3065a512e74c6a6 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 08:38:05 -0400 Subject: [PATCH 17/21] fixup!: space --- openedx_events/event_bus/tests/test_loader.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index b7d147aa..32ec3f30 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -185,4 +185,3 @@ def test_merge_configs_with_empty(self): dict_b = {} result = merge_publisher_configs(dict_a, dict_b) self.assertDictEqual(result, dict_a) - From 537ea446ec6d62627835bfb3ddf3c81f0906db9a Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 11:32:10 -0400 Subject: [PATCH 18/21] fixup!: comments from review --- openedx_events/apps.py | 12 +++++------ openedx_events/event_bus/__init__.py | 20 +++++++++---------- openedx_events/event_bus/tests/test_loader.py | 6 +++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/openedx_events/apps.py b/openedx_events/apps.py index 7221603a..0336eab1 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -14,20 +14,20 @@ def general_signal_handler(sender, signal, **kwargs): # pylint: disable=unused- """ Signal handler for publishing events to configured event bus. """ - event_type_publish_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) - # event_type_publish_configs should look something like + event_type_producer_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) + # event_type_producer_configs should look something like # { # "topic_a": { "event_key_field": "my.key.field", "enabled": True }, # "topic_b": { "event_key_field": "my.key.field", "enabled": False } # }" event_data = {key: kwargs.get(key) for key in signal.init_data} - for topic in event_type_publish_configs.keys(): - if event_type_publish_configs[topic]["enabled"] is True: + for topic in event_type_producer_configs.keys(): + if event_type_producer_configs[topic]["enabled"] is True: get_producer().send( signal=signal, topic=topic, - event_key_field=event_type_publish_configs[topic]["event_key_field"], + event_key_field=event_type_producer_configs[topic]["event_key_field"], event_data=event_data, event_metadata=kwargs["metadata"], ) @@ -53,7 +53,7 @@ def _get_validated_signal_config(self, event_type, configuration): Raises: ProducerConfigurationError: If configuration is not valid. """ - if not isinstance(configuration, dict) and not isinstance(configuration, dict): + if not isinstance(configuration, dict): raise ProducerConfigurationError( event_type=event_type, message="Configuration for event_types should be a dict" diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index eb3002ca..042a1711 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -186,26 +186,26 @@ def _reset_state(sender, **kwargs): # pylint: disable=unused-argument get_producer.cache_clear() -def merge_publisher_configs(publisher_config_original, publisher_config_overrides): +def merge_producer_configs(producer_config_original, producer_config_overrides): """ Merge two EVENT_BUS_PRODUCER_CONFIG maps. Arguments: - publisher_config_original: An EVENT_BUS_PRODUCER_CONFIG-structured map - publisher_config_overrides: An EVENT_BUS_PRODUCER_CONFIG-structured map + producer_config_original: An EVENT_BUS_PRODUCER_CONFIG-structured map + producer_config_overrides: An EVENT_BUS_PRODUCER_CONFIG-structured map Returns: A new EVENT_BUS_PRODUCER_CONFIG map created by combining the two maps. All event_type/topic pairs in - publisher_config_b are added to the publisher_config_a. If there is a conflict on whether a particular - event_type/topic pair is enabled, publisher_config_b wins out. + publisher_config_overrides are added to the publisher_config_original. If there is a conflict on whether a + particular event_type/topic pair is enabled, publisher_config_overrides wins out. """ - combined = copy.deepcopy(publisher_config_original) - for event_type, event_type_config_b in publisher_config_overrides.items(): + combined = copy.deepcopy(producer_config_original) + for event_type, event_type_config_overrides in producer_config_overrides.items(): event_type_config_combined = combined.get(event_type, {}) - for topic, topic_config_b in event_type_config_b.items(): + for topic, topic_config_overrides in event_type_config_overrides.items(): topic_config_combined = event_type_config_combined.get(topic, {}) - topic_config_combined['enabled'] = topic_config_b['enabled'] - topic_config_combined['event_key_field'] = topic_config_b['event_key_field'] + topic_config_combined['enabled'] = topic_config_overrides['enabled'] + topic_config_combined['event_key_field'] = topic_config_overrides['event_key_field'] event_type_config_combined[topic] = topic_config_combined combined[event_type] = event_type_config_combined return combined diff --git a/openedx_events/event_bus/tests/test_loader.py b/openedx_events/event_bus/tests/test_loader.py index 32ec3f30..ad742458 100644 --- a/openedx_events/event_bus/tests/test_loader.py +++ b/openedx_events/event_bus/tests/test_loader.py @@ -10,7 +10,7 @@ from django.test import override_settings from openedx_events.data import EventsMetadata -from openedx_events.event_bus import _try_load, get_producer, make_single_consumer, merge_publisher_configs +from openedx_events.event_bus import _try_load, get_producer, make_single_consumer, merge_producer_configs from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED @@ -155,7 +155,7 @@ def test_merge_configs(self): } } dict_b_copy = copy.deepcopy(dict_b) - result = merge_publisher_configs(dict_a, dict_b) + result = merge_producer_configs(dict_a, dict_b) self.assertDictEqual(result, { 'event_type_0': { 'topic_a': {'event_key_field': 'field', 'enabled': False}, @@ -183,5 +183,5 @@ def test_merge_configs_with_empty(self): } } dict_b = {} - result = merge_publisher_configs(dict_a, dict_b) + result = merge_producer_configs(dict_a, dict_b) self.assertDictEqual(result, dict_a) From e25bff8b0d085fec1b686fea4a4df6bcebd8c518 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 11:43:46 -0400 Subject: [PATCH 19/21] fixup!: comments from review --- openedx_events/apps.py | 2 +- openedx_events/event_bus/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx_events/apps.py b/openedx_events/apps.py index 0336eab1..6326e620 100644 --- a/openedx_events/apps.py +++ b/openedx_events/apps.py @@ -12,7 +12,7 @@ def general_signal_handler(sender, signal, **kwargs): # pylint: disable=unused-argument """ - Signal handler for publishing events to configured event bus. + Signal handler for producing events to configured event bus. """ event_type_producer_configs = getattr(settings, "EVENT_BUS_PRODUCER_CONFIG", {}).get(signal.event_type, {}) # event_type_producer_configs should look something like diff --git a/openedx_events/event_bus/__init__.py b/openedx_events/event_bus/__init__.py index 042a1711..6d508146 100644 --- a/openedx_events/event_bus/__init__.py +++ b/openedx_events/event_bus/__init__.py @@ -196,8 +196,8 @@ def merge_producer_configs(producer_config_original, producer_config_overrides): Returns: A new EVENT_BUS_PRODUCER_CONFIG map created by combining the two maps. All event_type/topic pairs in - publisher_config_overrides are added to the publisher_config_original. If there is a conflict on whether a - particular event_type/topic pair is enabled, publisher_config_overrides wins out. + producer_config_overrides are added to the producer_config_original. If there is a conflict on whether a + particular event_type/topic pair is enabled, producer_config_overrides wins out. """ combined = copy.deepcopy(producer_config_original) for event_type, event_type_config_overrides in producer_config_overrides.items(): From 4157019ec38583b70c84d574ba5820cf37c7a077 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 11:45:17 -0400 Subject: [PATCH 20/21] fixup!: comments from review --- openedx_events/tests/test_producer_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_events/tests/test_producer_config.py b/openedx_events/tests/test_producer_config.py index 61183827..05c4b8df 100644 --- a/openedx_events/tests/test_producer_config.py +++ b/openedx_events/tests/test_producer_config.py @@ -31,7 +31,7 @@ def setUp(self) -> None: @patch('openedx_events.apps.get_producer') def test_enabled_disabled_events(self, mock_producer): """ - Check whether XBLOCK_PUBLISHED is connected to the handler and the handler only publishes enabled events. + Check whether XBLOCK_PUBLISHED is connected to the handler and the handler only produces enabled events. Args: mock_producer: mock get_producer to inspect the arguments. From 03e23b8bc7bcacc0cb5321eb89348a94b9acd7e7 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Thu, 5 Oct 2023 12:00:49 -0400 Subject: [PATCH 21/21] fixup!: comments from review --- docs/decisions/0014-new-event-bus-producer-config.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/decisions/0014-new-event-bus-producer-config.rst b/docs/decisions/0014-new-event-bus-producer-config.rst index 0ba24b9f..4dc660d3 100644 --- a/docs/decisions/0014-new-event-bus-producer-config.rst +++ b/docs/decisions/0014-new-event-bus-producer-config.rst @@ -9,7 +9,7 @@ Status Context ******* -In a previous ADR, we set the structure for the event bus publishing configuration to a dictionary like the following: +In a previous ADR, we set the structure for the event bus producing configuration to a dictionary like the following: .. code-block:: python @@ -56,10 +56,10 @@ The new EVENT_BUS_PRODUCER_CONFIG will have the following configuration format: }, } -A new ``merge_publisher_configs`` method will be added to openedx-events.event_bus to make it easier to correctly determine the config map from multiple sources. +A new ``merge_producer_configs`` method will be added to openedx-events.event_bus to make it easier to correctly determine the config map from multiple sources. Consequences ************ -* As long as the implementing IDA calls ``merge_publisher_configs``, maintainers can add existing topics to new event_types without having to recreate the whole dictionary +* As long as the implementing IDA calls ``merge_producer_configs``, maintainers can add existing topics to new event_types without having to recreate the whole dictionary * There is no ambiguity about whether an event/topic pair is enabled or disabled