Merge pull request #97 from openedx/bmtcril/fix_missing_model_sink

fix: Event sink errors when enabled models are missing

platform_plugin_aspects/__init__.py

@@ -5,6 +5,6 @@
 import os
 from pathlib import Path
 
-__version__ = "0.11.2"
+__version__ = "0.11.3"
 
 ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__)))

platform_plugin_aspects/sinks/base_sink.py

@@ -5,6 +5,7 @@
 import csv
 import datetime
 import io
+import logging
 from collections import namedtuple
 
 import requests
@@ -342,7 +343,7 @@ def is_enabled(cls):
         """
         Return True if the sink is enabled, False otherwise
         """
-        enabled = getattr(
+        enabled_settings = getattr(
             settings,
             f"{WAFFLE_FLAG_NAMESPACE.upper()}_{cls.model.upper()}_ENABLED",
             False,
@@ -358,7 +359,15 @@ def is_enabled(cls):
             __name__,
         )
 
-        return enabled or waffle_flag.is_enabled()
+        enabled = enabled_settings or waffle_flag.is_enabled()
+
+        if enabled and not get_model(cls.model):
+            logging.warning(
+                f"Event Sink Model {cls.model} is not installed, but is enabled in settings or waffle flag."
+            )
+            enabled = False
+
+        return enabled
 
     @classmethod
     def get_sink_by_model_name(cls, model):

platform_plugin_aspects/sinks/tests/test_base_sink.py

@@ -270,16 +270,18 @@ def test_is_not_enabled_waffle(self, mock_waffle_flag_is_enabled):
         mock_waffle_flag_is_enabled.return_value = False
         self.assertEqual(self.child_sink.__class__.is_enabled(), False)
 
+    @patch("platform_plugin_aspects.sinks.base_sink.get_model")
     @patch("platform_plugin_aspects.sinks.base_sink.WaffleFlag.is_enabled")
-    def test_is_enabled_waffle(self, mock_waffle_flag_is_enabled):
+    def test_is_enabled_waffle(self, mock_waffle_flag_is_enabled, mock_get_model):
         """
         Test that is_enable() returns the correct data.
         """
         mock_waffle_flag_is_enabled.return_value = True
         self.assertEqual(self.child_sink.__class__.is_enabled(), True)
 
     @override_settings(EVENT_SINK_CLICKHOUSE_CHILD_MODEL_ENABLED=True)
-    def test_is_enabled(self):
+    @patch("platform_plugin_aspects.sinks.base_sink.get_model")
+    def test_is_enabled(self, mock_get_model):
         """
         Test that is_enable() returns the correct data.
         """

platform_plugin_aspects/sinks/tests/test_course_overview_sink.py

@@ -34,6 +34,7 @@
     registry=OrderedRegistry
 )
 @override_settings(EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEW_ENABLED=True)
+@patch("platform_plugin_aspects.sinks.base_sink.get_model")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_block")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.serialize_item")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.get_model")
@@ -47,6 +48,7 @@ def test_course_publish_success(
     mock_overview,
     mock_serialize_item,
     mock_get_tags,
+    mock_get_model,
 ):
     """
     Test of a successful end-to-end run.
@@ -110,13 +112,19 @@ def test_course_publish_success(
 @responses.activate(  # pylint: disable=unexpected-keyword-arg,no-value-for-parameter
     registry=OrderedRegistry
 )
+@patch("platform_plugin_aspects.sinks.base_sink.get_model")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.serialize_item")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.get_model")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_detached_xblock_types")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_modulestore")
 # pytest:disable=unused-argument
 def test_course_publish_clickhouse_error(
-    mock_modulestore, mock_detached, mock_overview, mock_serialize_item, caplog
+    mock_modulestore,
+    mock_detached,
+    mock_overview,
+    mock_serialize_item,
+    mock_get_model,
+    caplog,
 ):
     """
     Test the case where a ClickHouse POST fails.