refactor!: fail fast with invalid setting

The happy-path behavior for the CODE_OWNER_MAPPINGS
is untouched. This change now fails fast on
misconfigurations.

BREAKING CHANGE: Since edX.org is relying heavily
on the code_owner custom attributes, we will now
fail fast, rather than just logging an error, when
CODE_OWNER_MAPPINGS is misconfigured.

Author: robrap

CHANGELOG.rst

@@ -26,6 +26,11 @@ Added
 
 * Added new ``code_owner_theme`` and ``code_owner_squad`` custom attributes. This is useful in cases where the ``code_owner`` combines a theme and squad name, because monitoring can instead reference ``code_owner_squad`` to be resilient to theme name updates. For the decision doc, see edx_django_utils/monitoring/docs/decisions/0004-code-owner-theme-and-squad.rst.
 
+Updated
+-------
+
+* Misconfigurations of CODE_OWNER_MAPPINGS will now fail fast, rather than just logging. Although technically a breaking change, if CODE_OWNER_MAPPINGS is in use, it is probably correctly configured and this change should be a no-op.
+
 [3.16.0] - 2021-03-24
 ---------------------
 

edx_django_utils/monitoring/internal/code_owner/utils.py

@@ -29,7 +29,7 @@ def get_code_owner_from_module(module):
         return None
 
     code_owner_mappings = get_code_owner_mappings()
-    if code_owner_mappings is None:
+    if not code_owner_mappings:
         return None
 
     module_parts = module.split('.')
@@ -88,9 +88,7 @@ def get_code_owner_mappings():
     # .. setting_description: Used for monitoring and reporting of ownership. Use a
     #      dict with keys of code owner name and value as a list of dotted path
     #      module names owned by the code owner.
-    code_owner_mappings = getattr(settings, 'CODE_OWNER_MAPPINGS', None)
-    if code_owner_mappings is None:
-        return None
+    code_owner_mappings = getattr(settings, 'CODE_OWNER_MAPPINGS', {})
 
     try:
         for code_owner in code_owner_mappings:
@@ -102,10 +100,9 @@ def get_code_owner_mappings():
                 if optional_module_prefix_match:
                     path_without_prefix = path[optional_module_prefix_match.end():]
                     path_to_code_owner_mapping[path_without_prefix] = code_owner
-    except Exception as e:  # pylint: disable=broad-except
-        # will remove broad exceptions after ensuring all proper cases are covered
-        set_custom_attribute('deprecated_broad_except_get_code_owner_mappings', e.__class__)
-        log.exception('Error processing code_owner_mappings. {}'.format(e))
+    except TypeError as e:
+        log.exception('Error processing CODE_OWNER_MAPPINGS. {}'.format(e))
+        raise e
 
     _PATH_TO_CODE_OWNER_MAPPINGS = path_to_code_owner_mapping
     return _PATH_TO_CODE_OWNER_MAPPINGS

edx_django_utils/monitoring/tests/code_owner/test_middleware.py

@@ -285,11 +285,8 @@ def test_catch_all_with_errors(self, mock_set_custom_attribute, _):
     )
     def test_load_config_with_invalid_dict(self):
         request = RequestFactory().get('/test/')
-        self.middleware(request)
-        expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH['/test/']
-        self._assert_code_owner_custom_attributes(
-            mock_set_custom_attribute, path_module=expected_path_module,
-        )
+        with self.assertRaises(TypeError):
+            self.middleware(request)
 
     def _assert_code_owner_custom_attributes(self, mock_set_custom_attribute, expected_code_owner=None,
                                              path_module=None, has_path_error=False,

edx_django_utils/monitoring/tests/code_owner/test_utils.py

@@ -62,11 +62,12 @@ def test_code_owner_mapping_hits_and_misses(self, module, expected_owner):
     @override_settings(CODE_OWNER_MAPPINGS=['invalid_setting_as_list'])
     @patch('edx_django_utils.monitoring.internal.code_owner.utils.log')
     def test_code_owner_mapping_with_invalid_dict(self, mock_logger):
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(TypeError):
             get_code_owner_from_module('xblock')
-            mock_logger.exception.assert_called_with(
-                'Error processing code_owner_mappings.',
-            )
+
+        mock_logger.exception.assert_called_with(
+            'Error processing CODE_OWNER_MAPPINGS. list indices must be integers or slices, not str',
+        )
 
     def test_code_owner_mapping_with_no_settings(self):
         self.assertIsNone(get_code_owner_from_module('xblock'))