Merge pull request #745 from openedx/bmtcril/operator_dash_enhancements

Operator Dashboard Enhancements

tutoraspects/asset_command_helpers.py

@@ -123,7 +123,7 @@ def omit_templated_vars(self, content: dict, existing: dict):
         Omit templated variables from the content if they are not present in
         the existing file content.
         """
-        if not existing:
+        if not content or not existing:
             return
 
         for key in content.keys():
@@ -424,3 +424,92 @@ def check_asset_names(echo):
                 break
 
     echo(f"{warn} duplicate names detected.")
+
+
+def _get_all_chart_dataset_uuids():
+    """
+    Return the UUIDs of all datasets and charts in our file assets.
+    """
+    all_dataset_uuids = {}
+    all_chart_uuids = {}
+
+    # First get all known uuid's
+    for _, asset in _get_asset_files():
+        if "slice_name" in asset:
+            all_chart_uuids[asset["uuid"]] = asset["slice_name"]
+        elif "table_name" in asset:
+            all_dataset_uuids[asset["uuid"]] = asset["table_name"]
+
+    return all_dataset_uuids, all_chart_uuids
+
+
+def _get_used_chart_dataset_uuids():
+    """
+    Return the UUIDs of all datasets and charts actually used in our file assets.
+    """
+    used_dataset_uuids = set()
+    used_chart_uuids = set()
+
+    for _, asset in _get_asset_files():
+        if "dashboard_title" in asset:
+            filters = asset["metadata"].get("native_filter_configuration", [])
+
+            for filter_config in filters:
+                for filter_dataset in filter_config.get("target", {}).get(
+                    "datasetUuid", []
+                ):
+                    used_dataset_uuids.add(filter_dataset)
+
+            for pos in asset["position"]:
+                if pos.startswith("CHART-"):
+                    slice_uuid = asset["position"][pos]["meta"].get("uuid")
+
+                    if slice_uuid:
+                        used_chart_uuids.add(slice_uuid)
+
+        if "slice_name" in asset:
+            dataset_uuid = asset["dataset_uuid"]
+            used_dataset_uuids.add(dataset_uuid)
+
+    return used_dataset_uuids, used_chart_uuids
+
+
+def check_orphan_assets(echo):
+    """
+    Warn about any potentially unused assets.
+    """
+    echo("Looking for potentially orphaned assets...")
+
+    all_dataset_uuids, all_chart_uuids = _get_all_chart_dataset_uuids()
+    used_dataset_uuids, used_chart_uuids = _get_used_chart_dataset_uuids()
+
+    for k in used_dataset_uuids:
+        try:
+            all_dataset_uuids.pop(k)
+        except KeyError:
+            click.echo(
+                click.style(f"WARNING: Dataset {k} used nut not found!", fg="red")
+            )
+
+    # Remove the "Query performance" chart from the list, it's needed for
+    # the performance_metrics script, but not in any dashboard.
+    all_chart_uuids.pop("bb13bb31-c797-4ed3-a7f9-7825cc6dc482", None)
+
+    for k in used_chart_uuids:
+        try:
+            all_chart_uuids.pop(k)
+        except KeyError:
+            click.echo(click.style(f"WARNING: Chart {k} used nut not found!", fg="red"))
+
+    echo()
+
+    if all_dataset_uuids:
+        echo(click.style("Potentially unused datasets detected:", fg="yellow"))
+        echo("\n".join(sorted(all_dataset_uuids.values())))
+
+    if all_chart_uuids:
+        echo(click.style("Potentially unused charts detected:", fg="yellow"))
+        echo("\n".join(sorted(all_chart_uuids.values())))
+
+    if not all_dataset_uuids and not all_chart_uuids:
+        echo(f"{len(all_chart_uuids) + len(all_dataset_uuids)} orphans detected.")

tutoraspects/commands_v1.py

@@ -11,6 +11,7 @@
 
 from tutoraspects.asset_command_helpers import (
     check_asset_names,
+    check_orphan_assets,
     import_superset_assets,
     deduplicate_superset_assets,
     SupersetCommandError,
@@ -114,7 +115,7 @@ def alembic(command: string) -> list[tuple[str, str]]:
     ]
 
 
-# Ex: "tutor local do import_assets "
+# Ex: "tutor local do import-assets "
 @click.command(context_settings={"ignore_unknown_options": True})
 def import_assets() -> list[tuple[str, str]]:
     """
@@ -317,6 +318,8 @@ def check_superset_assets():
 
     click.echo()
     check_asset_names(click.echo)
+    click.echo()
+    check_orphan_assets(click.echo)
 
     click.echo()
     click.echo(

tutoraspects/patches/openedx-dev-dockerfile-post-python-requirements

@@ -1,4 +1,4 @@
 RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \
     pip install "platform-plugin-aspects==v0.7.2"
 RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \
-    pip install "edx-event-routing-backends==v9.0.0"
+    pip install "edx-event-routing-backends==v9.0.1"

tutoraspects/patches/openedx-dockerfile-post-python-requirements

@@ -1,4 +1,4 @@
 RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \
     pip install "platform-plugin-aspects==v0.7.2"
 RUN --mount=type=cache,target=/openedx/.cache/pip,sharing=shared \
-    pip install "edx-event-routing-backends==v9.0.0"
+    pip install "edx-event-routing-backends==v9.0.1"

tutoraspects/plugin.py

@@ -334,7 +334,7 @@
         # For now we are pulling this from github, which should allow maximum
         # flexibility for forking, running branches, specific versions, etc.
         ("DBT_REPOSITORY", "https://github.com/openedx/aspects-dbt"),
-        ("DBT_BRANCH", "v3.19.2"),
+        ("DBT_BRANCH", "v3.20.0"),
         ("DBT_SSH_KEY", ""),
         ("DBT_STATE_DIR", "/app/aspects/dbt_state/"),
         # This is the name of the database dbt will write to

tutoraspects/templates/aspects/apps/aspects/dbt/profiles.yml

@@ -22,4 +22,9 @@ aspects: # this needs to match the profile in your dbt_project.yml file
       check_exchange: false
       sync_request_timeout: 5
       compress_block_size: 1048576
+
+      # Without this dbt queries populating tables can be killed for using too much memory
+      custom_settings:
+        memory_overcommit_ratio_denominator_for_user: 0
+
       {{ patch("dbt-profiles") | indent(6)}}

tutoraspects/templates/aspects/apps/aspects/scripts/dbt.sh

@@ -1,5 +1,8 @@
 #!/usr/bin/env bash
 
+
+set -eo pipefail
+
 ## WARNING: If you modify this block, make sure to also update the
 ##          corresponding block in the init-aspects.sh file.
 
@@ -18,10 +21,9 @@ echo "Installing aspects-dbt"
 echo "git clone -b {{ DBT_BRANCH }} {{ DBT_REPOSITORY }}"
 git clone -b {{ DBT_BRANCH }} {{ DBT_REPOSITORY }} aspects-dbt
 
-cd aspects-dbt || exit
+cd aspects-dbt
 
 echo "Installing dbt python requirements"
-pip install -r /app/aspects/dbt/requirements.txt
 pip install -r ./requirements.txt
 
 export ASPECTS_EVENT_SINK_DATABASE={{ASPECTS_EVENT_SINK_DATABASE}}

tutoraspects/templates/aspects/apps/superset/pythonpath/create_assets.py

@@ -12,21 +12,20 @@
 from pathlib import Path
 
 from superset import security_manager
-from superset.examples.utils import load_configs_from_directory
 from superset.extensions import db
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
 from superset.connectors.sqla.models import SqlaTable
 from superset.utils.database import get_or_create_db
 from superset.models.embedded_dashboard import EmbeddedDashboard
+
+from pythonpath.create_assets_utils import load_configs_from_directory
 from pythonpath.localization import get_translation
 from pythonpath.create_row_level_security import create_rls_filters
 
 
 logger = logging.getLogger("create_assets")
-
 BASE_DIR = "/app/assets/superset"
-
 ASSET_FOLDER_MAPPING = {
     "dashboard_title": "dashboards",
     "slice_name": "charts",
@@ -116,7 +115,7 @@ def write_asset_to_file(asset, asset_name, folder, file_name, roles):
                 asset, asset_name, folder, locale, roles
             )
 
-            # Clean up old dashboard
+            # Clean up old localized dashboards
             if folder == "dashboards":
                 dashboard_slug = updated_asset["slug"]
                 dashboard = db.session.query(Dashboard).filter_by(slug=dashboard_slug).first()
@@ -132,6 +131,7 @@ def write_asset_to_file(asset, asset_name, folder, file_name, roles):
     if dashboard_roles:
         roles[asset["uuid"]] = [security_manager.find_role("Admin")]
 
+    # Clean up old un-localized dashboard
     dashboard_slug = asset.get("slug")
     if dashboard_slug:
         dashboard = db.session.query(Dashboard).filter_by(slug=dashboard_slug).first()
@@ -263,8 +263,8 @@ def update_dashboard_roles(roles):
             owners.append(user)
 
     for dashboard_uuid, role_ids in roles.items():
-        dashboard = db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
         logger.info(f"Importing dashboard roles: {dashboard_uuid} - {role_ids}")
+        dashboard = db.session.query(Dashboard).filter_by(uuid=dashboard_uuid).one()
         dashboard.roles = role_ids
         if owners:
             dashboard.owners = owners

tutoraspects/templates/aspects/apps/superset/pythonpath/create_assets_utils.py

@@ -0,0 +1,80 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+
+# This is taken from Superset's example utils:
+# https://github.com/apache/superset/blob/7081a0e73d872332a1a63727c9ff7a22159018bb/superset/examples/utils.py#L73
+
+# Changes were added to mock out the security manager, since the other options
+# are to either replicate asset import entirely or to use the "examples"
+# version, which skips a variety of other checks.
+import logging
+import yaml
+from pathlib import Path
+
+from flask import g
+from superset.commands.exceptions import CommandInvalidError
+from superset.commands.importers.v1.assets import ImportAssetsCommand
+from superset.commands.importers.v1.utils import METADATA_FILE_NAME
+from superset.commands.database.importers.v1.utils import security_manager
+
+logger = logging.getLogger(__name__)
+
+# This logger is really spammy
+model_helper_logger = logging.getLogger("superset.models.helpers")
+model_helper_logger.setLevel(logging.WARNING)
+
+
+YAML_EXTENSIONS = {".yaml", ".yml"}
+
+
+
+def load_configs_from_directory(
+    root: Path, overwrite: bool = True, force_data: bool = False
+) -> None:
+    """
+    Load all the examples from a given directory.
+    """
+    contents: dict[str, str] = {}
+    queue = [root]
+    while queue:
+        path_name = queue.pop()
+        if path_name.is_dir():
+            queue.extend(path_name.glob("*"))
+        elif path_name.suffix.lower() in YAML_EXTENSIONS:
+            with open(path_name) as fp:
+                contents[str(path_name.relative_to(root))] = fp.read()
+
+    # removing "type" from the metadata allows us to import any exported model
+    # from the unzipped directory directly
+    metadata = yaml.load(contents.get(METADATA_FILE_NAME, "{}"), Loader=yaml.Loader)
+    if "type" in metadata:
+        del metadata["type"]
+    contents[METADATA_FILE_NAME] = yaml.dump(metadata)
+
+    # Force our use to the admin user to prevent errors on import
+    g.user = security_manager.find_user(username="{{SUPERSET_ADMIN_USERNAME}}")
+
+    command = ImportAssetsCommand(
+        contents,
+        overwrite=overwrite,
+        force_data=force_data,
+    )
+    try:
+        command.run()
+    except CommandInvalidError as ex:
+        logger.error("An error occurred: %s", ex.normalized_messages())

tutoraspects/templates/aspects/apps/superset/pythonpath/performance_metrics.py

@@ -160,7 +160,7 @@ def get_query_log_from_clickhouse(report, query_contexts, print_sql):
     """
     Get the query log from clickhouse and print the results.
     """
-    # This corresponsds to the "Query Performance" chart in Superset
+    # This corresponds to the "Query Performance" chart in Superset
     chart_uuid = "bb13bb31-c797-4ed3-a7f9-7825cc6dc482"
 
     slice = db.session.query(Slice).filter(Slice.uuid == chart_uuid).one()

tutoraspects/templates/aspects/apps/superset/pythonpath/superset_config_docker.py

@@ -91,7 +91,7 @@
     "EMBEDDABLE_CHARTS": True,
     "EMBEDDED_SUPERSET": True,
     "ENABLE_TEMPLATE_PROCESSING": True,
-    "TAGGING_SYSTEM": True,
+    "TAGGING_SYSTEM": False,
 }
 
 # Add this custom template processor which returns the list of courses the current user can access

tutoraspects/templates/aspects/apps/superset/scripts/import-assets.sh

@@ -1,3 +1,7 @@
+#!/usr/bin/env bash
+
+set -eo pipefail
+
 mkdir -p /app/assets/
 cd /app/assets/
 rm -rf superset