Skip to content

Commit

Permalink
fix: handle missing keys when merging producer configs
Browse files Browse the repository at this point in the history
  • Loading branch information
Rebecca Graber authored Oct 31, 2023
1 parent 6e7dbd2 commit 8caa658
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 22 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ Change Log
Unreleased
----------

[9.0.1] - 2023-10-31
--------------------
Changed
~~~~~~~
* Fixed key error in merging event producer configs. Previously, setting only one of `enabled` or `event_key_field` would result in a KeyError being thrown

[9.0.0] - 2023-10-04
--------------------
Changed
Expand Down
2 changes: 1 addition & 1 deletion openedx_events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
more information about the project.
"""

__version__ = "9.0.0"
__version__ = "9.0.1"
8 changes: 6 additions & 2 deletions openedx_events/event_bus/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,12 @@ def merge_producer_configs(producer_config_original, producer_config_overrides):
event_type_config_combined = combined.get(event_type, {})
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_overrides['enabled']
topic_config_combined['event_key_field'] = topic_config_overrides['event_key_field']
enabled_override = topic_config_overrides.get('enabled', None)
event_key_field_override = topic_config_overrides.get('event_key_field', None)
if enabled_override is not None:
topic_config_combined['enabled'] = enabled_override
if event_key_field_override is not None:
topic_config_combined['event_key_field'] = event_key_field_override
event_type_config_combined[topic] = topic_config_combined
combined[event_type] = event_type_config_combined
return combined
53 changes: 34 additions & 19 deletions openedx_events/event_bus/tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,10 @@ def test_default_does_nothing(self):


class TestSettings(TestCase):
def test_merge_configs(self):
dict_a = {

def setUp(self) -> None:
super().setUp()
self.base_config = {
'event_type_0': {
'topic_a': {'event_key_field': 'field', 'enabled': True},
'topic_b': {'event_key_field': 'field', 'enabled': True}
Expand All @@ -140,25 +142,27 @@ def test_merge_configs(self):
'topic_c': {'event_key_field': 'field', 'enabled': True},
}
}

def test_merge_configs(self):
# for ensuring we didn't change the original dict
dict_a_copy = copy.deepcopy(dict_a)
dict_b = {
base_copy = copy.deepcopy(self.base_config)
overrides = {
'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
# disable an existing event/topic pairing and change the key field
'topic_a': {'event_key_field': 'new_field', 'enabled': False},
# add a new topic to an existing event_type
'topic_d': {'event_key_field': 'field', 'enabled': True},
},
# add a new event_type
'event_type_2': {
'topic_e': {'event_key_field': 'field', 'enabled': True},
}
}
dict_b_copy = copy.deepcopy(dict_b)
result = merge_producer_configs(dict_a, dict_b)
overrides_copy = copy.deepcopy(overrides)
result = merge_producer_configs(self.base_config, overrides)
self.assertDictEqual(result, {
'event_type_0': {
'topic_a': {'event_key_field': 'field', 'enabled': False},
'topic_a': {'event_key_field': 'new_field', 'enabled': False},
'topic_b': {'event_key_field': 'field', 'enabled': True},
'topic_d': {'event_key_field': 'field', 'enabled': True},
},
Expand All @@ -169,19 +173,30 @@ 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)
self.assertDictEqual(self.base_config, base_copy)
self.assertDictEqual(overrides, overrides_copy)

def test_merge_configs_with_empty(self):
dict_a = {
overrides = {}
result = merge_producer_configs(self.base_config, overrides)
self.assertDictEqual(result, self.base_config)

def test_merge_configs_with_partial(self):
overrides = {
'event_type_0': {
'topic_a': {'event_key_field': 'field', 'enabled': True},
'topic_b': {'event_key_field': 'field', 'enabled': True}
# no override for 'event_key_field'
'topic_a': {'enabled': False},
# no override for 'enabled'
'topic_b': {'event_key_field': 'new_field'}
}
}
result = merge_producer_configs(self.base_config, overrides)
self.assertDictEqual(result, {
'event_type_0': {
'topic_a': {'event_key_field': 'field', 'enabled': False},
'topic_b': {'event_key_field': 'new_field', 'enabled': True}
},
'event_type_1': {
'topic_c': {'event_key_field': 'field', 'enabled': True},
}
}
dict_b = {}
result = merge_producer_configs(dict_a, dict_b)
self.assertDictEqual(result, dict_a)
})

0 comments on commit 8caa658

Please sign in to comment.