From 8b6fd83beeee1a9bd64c73e2e876bfb80aaa999f Mon Sep 17 00:00:00 2001 From: Ty Hob Date: Wed, 23 Jul 2025 15:21:01 -0400 Subject: [PATCH 1/3] fix: Improve error messages, add in-context setting This top level settings lets us decide which dashboards to include when asking for Superset guest tokens, which fixes issues in OeX releases before Sumac and where newer versions of platform-plugin-aspects are used with older versions of tutor-contrib-aspects. --- README.rst | 10 +++++++++- platform_plugin_aspects/__init__.py | 2 +- platform_plugin_aspects/settings/common.py | 4 ++++ platform_plugin_aspects/settings/production.py | 4 ++++ platform_plugin_aspects/tests/test_xblock.py | 7 +++++-- platform_plugin_aspects/utils.py | 15 ++++++--------- platform_plugin_aspects/views.py | 9 ++++++++- test_settings.py | 1 + 8 files changed, 38 insertions(+), 14 deletions(-) diff --git a/README.rst b/README.rst index 339f9778..bdc4cec6 100644 --- a/README.rst +++ b/README.rst @@ -12,7 +12,7 @@ Version Compatibility ********************* - ``platform-plugin-aspects`` version 1.x: Nutmeg to Sumac -- ``platform-plugin-aspects`` version 2.x: Redwood and above +- ``platform-plugin-aspects`` version 2.x: Redwood and above (with in-context metrics turned off) Sinks ***** @@ -156,6 +156,14 @@ superset pipeline into the filter as follows: - `SUPERSET_DASHBOARD_LOCALES` - This setting is used to configure the available locales for the dashboards. The configuration is a list of supported locales by `Aspects`_. +- `ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS` - This setting turns on and off the in-context + metrics feature. It must be turned off in Open edX releases before Sumac and when using + tutor-contrib-aspects before v2.2.0 as those dashboards will not exist, causing errors in + the embedded Instructor Dashboards. + +- `ASPECTS_IN_CONTEXT_DASHBOARDS` - This setting mirrors the `ASPECTS_INSTRUCTOR_DASHBOARDS` but + with additional keys used for filtering the boards to specific courses and blocks. + Event Sink Configuration ======================== diff --git a/platform_plugin_aspects/__init__.py b/platform_plugin_aspects/__init__.py index d5f7234d..c13bb705 100644 --- a/platform_plugin_aspects/__init__.py +++ b/platform_plugin_aspects/__init__.py @@ -5,6 +5,6 @@ import os from pathlib import Path -__version__ = "1.1.0" +__version__ = "1.1.1" ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__))) diff --git a/platform_plugin_aspects/settings/common.py b/platform_plugin_aspects/settings/common.py index 89104014..5d769a03 100644 --- a/platform_plugin_aspects/settings/common.py +++ b/platform_plugin_aspects/settings/common.py @@ -105,6 +105,10 @@ def plugin_settings(settings): }, } + # This is on by default here, off by default in tutor-contrib-aspects until we only + # support versions Sumac and beyond + settings.ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS = True + settings.ASPECTS_IN_CONTEXT_DASHBOARDS = { "course": { "name": _("Course"), diff --git a/platform_plugin_aspects/settings/production.py b/platform_plugin_aspects/settings/production.py index 9feec029..ec834bf8 100644 --- a/platform_plugin_aspects/settings/production.py +++ b/platform_plugin_aspects/settings/production.py @@ -28,6 +28,10 @@ def plugin_settings(settings): "EVENT_SINK_CLICKHOUSE_PII_MODELS", settings.EVENT_SINK_CLICKHOUSE_PII_MODELS, ) + settings.ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS = settings.ENV_TOKENS.get( + "ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS", + settings.ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS, + ) settings.ASPECTS_IN_CONTEXT_DASHBOARDS = settings.ENV_TOKENS.get( "ASPECTS_IN_CONTEXT_DASHBOARDS", settings.ASPECTS_IN_CONTEXT_DASHBOARDS, diff --git a/platform_plugin_aspects/tests/test_xblock.py b/platform_plugin_aspects/tests/test_xblock.py index 3fba4dd5..5f5afb89 100644 --- a/platform_plugin_aspects/tests/test_xblock.py +++ b/platform_plugin_aspects/tests/test_xblock.py @@ -2,6 +2,7 @@ """ Test basic SupersetXBlock display function """ + import json from unittest import TestCase from unittest.mock import Mock, patch @@ -154,6 +155,8 @@ def test_guest_token_handler_failed(self, mock_generate_guest_token): assert response.status_code == 500 data = json.loads(response.body.decode("utf-8")) - assert data.get("error") == ( - "Unable to fetch Superset guest token, mostly likely due to invalid settings.SUPERSET_CONFIG" + assert ( + "Unable to fetch Superset guest token, mostly likely due to invalid " + "settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested" + in data.get("error") ) diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py index 2e64846b..c82732f1 100644 --- a/platform_plugin_aspects/utils.py +++ b/platform_plugin_aspects/utils.py @@ -158,22 +158,19 @@ def generate_guest_token(user, course, dashboards, filters) -> str: except HTTPError as err: # Superset server errors sometimes come with messages, so log the response. logger.error( - f"{err.response.status_code} {err.response.json()} for url: {err.response.url}, data: {data}" + f"{err.response.status_code} {err.response.json()} for url: " + f"{err.response.url}, data: {data}" ) raise ImproperlyConfigured( - _( - "Unable to fetch Superset guest token, " - "Superset server error {server_response}" - ).format(server_response=err.response.json()) + f"Unable to fetch Superset guest token, Superset server error: {err.response.json()}", ) from err except Exception as exc: logger.error(exc) raise ImproperlyConfigured( - _( - "Unable to fetch Superset guest token, " - "mostly likely due to invalid settings.SUPERSET_CONFIG" - ) + "Unable to fetch Superset guest token, mostly likely due to invalid " + "settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested does not " + f"exist in Superset. Requested UUIDs: {resources}" ) from exc diff --git a/platform_plugin_aspects/views.py b/platform_plugin_aspects/views.py index 2ecd58c0..9afdd2e7 100644 --- a/platform_plugin_aspects/views.py +++ b/platform_plugin_aspects/views.py @@ -128,7 +128,14 @@ def get(self, request, *args, **kwargs): course = self.get_object() dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS.copy() - dashboards.extend(settings.ASPECTS_IN_CONTEXT_DASHBOARDS.values()) + + # Only include these dashboards if in-context metrics are on, + # otherwise this call will always fail in older releases without + # admin intervention. This can be removed when we stop supporting + # < Sumac. + if settings.ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS: + dashboards.extend(settings.ASPECTS_IN_CONTEXT_DASHBOARDS.values()) + extra_filters_format = settings.SUPERSET_EXTRA_FILTERS_FORMAT try: diff --git a/test_settings.py b/test_settings.py index b3968938..03a8b45f 100644 --- a/test_settings.py +++ b/test_settings.py @@ -99,6 +99,7 @@ } ] +ASPECTS_ENABLE_STUDIO_IN_CONTEXT_METRICS = True ASPECTS_IN_CONTEXT_DASHBOARDS = { "course": { "slug": "in-context-course", From 4f9d7cf98e9b0fad52ad3210b9d07a941029d4aa Mon Sep 17 00:00:00 2001 From: Ty Hob Date: Tue, 29 Jul 2025 16:20:07 -0400 Subject: [PATCH 2/3] fix: Log guest token errors explicitly Previously the error messages sent upstream were eaten, making debugging very difficult. This ensures the correct messages are logged. --- platform_plugin_aspects/tests/test_xblock.py | 3 +- platform_plugin_aspects/utils.py | 30 ++++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/platform_plugin_aspects/tests/test_xblock.py b/platform_plugin_aspects/tests/test_xblock.py index 5f5afb89..477b611a 100644 --- a/platform_plugin_aspects/tests/test_xblock.py +++ b/platform_plugin_aspects/tests/test_xblock.py @@ -156,7 +156,6 @@ def test_guest_token_handler_failed(self, mock_generate_guest_token): assert response.status_code == 500 data = json.loads(response.body.decode("utf-8")) assert ( - "Unable to fetch Superset guest token, mostly likely due to invalid " - "settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested" + "Error trying to fetch Superset guest token, mostly likely due to invalid" in data.get("error") ) diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py index c82732f1..272c5e29 100644 --- a/platform_plugin_aspects/utils.py +++ b/platform_plugin_aspects/utils.py @@ -155,23 +155,23 @@ def generate_guest_token(user, course, dashboards, filters) -> str: token = response.json().get("token") return token - except HTTPError as err: - # Superset server errors sometimes come with messages, so log the response. - logger.error( - f"{err.response.status_code} {err.response.json()} for url: " - f"{err.response.url}, data: {data}" - ) - raise ImproperlyConfigured( - f"Unable to fetch Superset guest token, Superset server error: {err.response.json()}", - ) from err + # We manually log the error message here because ImproperlyConfigured is eaten up the stack + except HTTPError as exc: + err = f"""Unable to fetch Superset guest token, mostly likely due to invalid + settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested does not + exist in Superset.\n + {exc.response.status_code} {exc.response.json()} for url: + {exc.response.url}, data: {data}\n\n dashboards: {dashboards}""" + + logger.error(err) + raise ImproperlyConfigured(err) from exc except Exception as exc: - logger.error(exc) - raise ImproperlyConfigured( - "Unable to fetch Superset guest token, mostly likely due to invalid " - "settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested does not " - f"exist in Superset. Requested UUIDs: {resources}" - ) from exc + err = f"""Error trying to fetch Superset guest token, mostly likely due to invalid + settings.SUPERSET_CONFIG or because one of the dashboard UUIDs requested does not + exist in Superset. data: {data} \n\n exception: {exc}""" + logger.error(err) + raise ImproperlyConfigured(err) from exc def _fix_service_url(url: str) -> str: From f899423cbf9b66ff23d27a864d19e4a993b04f5e Mon Sep 17 00:00:00 2001 From: Ty Hob / Brian Mesick <112640379+bmtcril@users.noreply.github.com> Date: Wed, 30 Jul 2025 17:18:03 +0000 Subject: [PATCH 3/3] docs: Update README.rst Co-authored-by: Sara Burns --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index bdc4cec6..8fc8f149 100644 --- a/README.rst +++ b/README.rst @@ -12,7 +12,7 @@ Version Compatibility ********************* - ``platform-plugin-aspects`` version 1.x: Nutmeg to Sumac -- ``platform-plugin-aspects`` version 2.x: Redwood and above (with in-context metrics turned off) +- ``platform-plugin-aspects`` version 2.x: Redwood (with in-context metrics turned off), or Sumac and above Sinks *****