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

fix: Event sink errors when enabled models are missing #97

Merged
merged 3 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion platform_plugin_aspects/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)))
13 changes: 11 additions & 2 deletions platform_plugin_aspects/sinks/base_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import csv
import datetime
import io
import logging
from collections import namedtuple

import requests
Expand Down Expand Up @@ -342,7 +343,7 @@
"""
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,
Expand All @@ -358,7 +359,15 @@
__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

Check failure on line 368 in platform_plugin_aspects/sinks/base_sink.py

View workflow job for this annotation

GitHub Actions / tests (ubuntu-24.04, 3.11, django42)

Missing coverage

Missing coverage on lines 365-368

return enabled

@classmethod
def get_sink_by_model_name(cls, model):
Expand Down
6 changes: 4 additions & 2 deletions platform_plugin_aspects/sinks/tests/test_base_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Loading