Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: new event bus config format #272

Merged
merged 24 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions openedx_events/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for topic in event_type_producer_configs.keys():
if event_type_producer_configs[topic]["enabled"] is True:
for topic, config in event_type_producer_configs.items():
if config["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"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
event_key_field=event_type_producer_configs[topic]["event_key_field"],
event_key_field=config["event_key_field"],

event_data=event_data,
event_metadata=kwargs["metadata"],
)
Expand All @@ -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"
Expand Down
20 changes: 10 additions & 10 deletions openedx_events/event_bus/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: renaming these variables to original and overrides would make it easier to understand as producer_config prefix should not be needed.

"""
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
robrap marked this conversation as resolved.
Show resolved Hide resolved
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
6 changes: 3 additions & 3 deletions openedx_events/event_bus/tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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)