From 80dad15458317ae512b901c592b2ed26f5046632 Mon Sep 17 00:00:00 2001 From: busya Date: Tue, 4 Aug 2026 08:46:22 +0700 Subject: [PATCH] fix(maintenance): render banner as native MARKDOWN element, not a chart Two defects fixed: - ensure_banner_chart created an orphan 'Maintenance Banner' markdown chart (polluted Charts menu and dashboard exports). The banner is now a native MARKDOWN element in position_json; chart_id is a synthetic layout key. - insert_banner_markdown_at_top blindly targeted GRID_ID; on ROOT->TABS dashboards (FI-0085) GRID_ID is orphaned and the banner never rendered. The layout is now normalized to ROOT->GRID_ID->[ROW-banner, ...] with recursive parents update, matching the proven-working manual example. Review-driven hardening: - liveness check verifies reachability from ROOT (children graph), so dashboards corrupted by the old bug self-heal on the next start. - insert removes stale ROW-banner-*/MARKDOWN-banner-* keys (single banner). - decision memory (@RATIONALE/@REJECTED) added to both modules. - new ops script scripts/cleanup_maintenance_banner_charts.py (dry-run by default) deletes already-created bogus banner charts in prod. --- .../src/core/superset_client/_layout_utils.py | 292 +++++++++++++++--- .../services/maintenance/_banner_renderer.py | 10 +- .../services/maintenance/_chart_manager.py | 123 ++++---- .../maintenance/test_chart_manager.py | 100 ++---- backend/tests/test_core/test_layout_utils.py | 137 ++++++++ backend/tests/test_maintenance_service.py | 42 +-- scripts/cleanup_maintenance_banner_charts.py | 203 ++++++++++++ 7 files changed, 702 insertions(+), 205 deletions(-) create mode 100644 scripts/cleanup_maintenance_banner_charts.py diff --git a/backend/src/core/superset_client/_layout_utils.py b/backend/src/core/superset_client/_layout_utils.py index fb9c2d78b..dc9714aba 100644 --- a/backend/src/core/superset_client/_layout_utils.py +++ b/backend/src/core/superset_client/_layout_utils.py @@ -4,6 +4,15 @@ # @LAYER Infrastructure # @RELATION DEPENDS_ON -> [EXT:Python:json] # @RELATION DEPENDS_ON -> [Core.DashboardsWrite.SupersetDashboardsWriteMixin] +# @RATIONALE Banner insertion must work for both ROOT -> GRID_ID and ROOT -> TABS +# (GRID orphaned) layouts; the tree is normalized to ROOT -> GRID_ID so the banner +# always renders. Key presence in position_json is NOT enough — reachability from +# ROOT is verified because the old bug left banners under an orphaned GRID_ID. +# @REJECTED Inserting the banner ROW into GRID_ID unconditionally was rejected — +# dashboards whose rendered root is a TABS element never showed the banner +# (production dashboard FI-0085). +# @REJECTED Trusting the `parents` field was rejected — Superset rebuilds it from +# `children` on hydrate; the children graph is the rendering source of truth. # @RATIONALE Extracted from SupersetDashboardsWriteMixin to stay under INV_7 400 LOC. import json @@ -77,27 +86,108 @@ def _generate_banner_id(chart_id: int) -> tuple[str, str]: # #endregion Core.LayoutUtils.GenerateBannerId -# #region Core.LayoutUtils.InsertBannerMarkdownAtTop [C:2] [TYPE Function] +# #region Core.LayoutUtils.ShiftExistingDown [C:2] [TYPE Function] # @ingroup Core -# @BRIEF Insert a ROW + native MARKDOWN pair at the top of the dashboard grid. -# Creates a ROW entry connected to GRID_ID, and a MARKDOWN child inside it. -# Shifts all existing grid ROWs down by 2 rows. -# @PRE position_json is a mutable dict. content is the HTML/markdown string. -# chart_id is the database chart_id for key generation. -# @POST position_json is mutated; ROW + MARKDOWN inserted at top of GRID. -def insert_banner_markdown_at_top( - position_json: dict, chart_id: int, content: str -) -> dict: - row_key, md_key = _generate_banner_id(chart_id) - - # Shift existing ROW y-positions down - for key, value in list(position_json.items()): +# @BRIEF Shift every positioned element's y-coordinate down by an offset to make +# room for the banner at the top. +# @PRE position_json is a mutable dict. +# @POST position_json mutated; y-coordinates increased. +def _shift_existing_down(position_json: dict, offset: int = 2) -> None: + for _key, value in list(position_json.items()): if isinstance(value, dict): meta = value.get("meta", {}) if isinstance(meta, dict): cur_y = meta.get("y", 0) if isinstance(cur_y, (int, float)): - meta["y"] = cur_y + 2 + meta["y"] = cur_y + offset +# #endregion Core.LayoutUtils.ShiftExistingDown + + +# #region Core.LayoutUtils.PrependRowToGrid [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Prepend a banner ROW key to the GRID_ID children list (dedup-first). +# @PRE position_json is a mutable dict. +# @POST GRID_ID children list starts with row_key. +def _prepend_row_to_grid(position_json: dict, row_key: str) -> None: + grid = position_json.get("GRID_ID") + if not isinstance(grid, dict): + return + grid_children = grid.get("children") + if not isinstance(grid_children, list): + return + if row_key in grid_children: + grid_children.remove(row_key) + grid_children.insert(0, row_key) +# #endregion Core.LayoutUtils.PrependRowToGrid + + +# #region Core.LayoutUtils.RemoveStaleBanners [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Remove every existing banner ROW/MARKDOWN node from the layout (DB guarantees +# at most one active banner per dashboard, so any other banner key is stale). Used on +# insert to auto-heal layouts corrupted by earlier bugs (e.g. orphaned GRID banners). +# @PRE position_json is a mutable dict. +# @POST position_json mutated; no ROW-banner-* / MARKDOWN-banner-* keys remain. +def _remove_stale_banners(position_json: dict) -> None: + stale_keys = [ + key + for key in position_json + if key.startswith("MARKDOWN-banner-") or key.startswith("ROW-banner-") + ] + for key in stale_keys: + position_json.pop(key, None) + _remove_row_from_children(position_json, "GRID_ID", key) + _remove_row_from_children(position_json, "ROOT_ID", key) +# #endregion Core.LayoutUtils.RemoveStaleBanners + + +# #region Core.LayoutUtils.IsKeyReachableFromRoot [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Check whether a layout key is reachable from ROOT_ID via the children graph. +# Iterative walk (cycle-safe). If ROOT_ID is missing, falls back to key presence. +# @PRE position_json is a dict. +# @POST Returns True when key is in the rendered tree. +def _is_key_reachable_from_root(position_json: dict, key: str) -> bool: + root = position_json.get("ROOT_ID") + if not isinstance(root, dict): + return key in position_json + seen: set[str] = set() + stack = list(root.get("children") or []) + while stack: + node_key = stack.pop() + if node_key in seen: + continue + seen.add(node_key) + if node_key == key: + return True + node = position_json.get(node_key) + if isinstance(node, dict): + children = node.get("children") + if isinstance(children, list): + stack.extend(children) + return False +# #endregion Core.LayoutUtils.IsKeyReachableFromRoot + + +# #region Core.LayoutUtils.InsertBannerMarkdownAtTop [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Insert a ROW + native MARKDOWN pair at the top of the dashboard grid. +# Creates a ROW entry connected to GRID_ID, and a MARKDOWN child inside it. +# Shifts all existing grid ROWs down by 2 rows. Handles both ROOT -> GRID_ID and +# ROOT -> TABS (GRID orphaned) layouts. +# @PRE position_json is a mutable dict. content is the HTML/markdown string. +# chart_id is the database chart_id for key generation. +# @POST position_json is mutated; ROW + MARKDOWN inserted at top of the rendered grid. +def insert_banner_markdown_at_top( + position_json: dict, chart_id: int, content: str +) -> dict: + row_key, md_key = _generate_banner_id(chart_id) + + # Remove any stale banner nodes (own + leftovers from previous banners/bugs) + _remove_stale_banners(position_json) + + # Shift existing content y-positions down to make room for the banner + _shift_existing_down(position_json) # Add MARKDOWN entry with adaptive height height = _estimate_markdown_height(content) @@ -126,19 +216,97 @@ def insert_banner_markdown_at_top( }, } - # Insert ROW at beginning of GRID_ID children - grid = position_json.get("GRID_ID") - if grid and isinstance(grid, dict): - grid_children = grid.get("children", []) - if isinstance(grid_children, list): - if row_key in grid_children: - grid_children.remove(row_key) - grid_children.insert(0, row_key) + root = position_json.get("ROOT_ID") + if not isinstance(root, dict): + # Degenerate layout without ROOT_ID: still prepend to GRID if present. + _prepend_row_to_grid(position_json, row_key) + return position_json + root_children = root.get("children") + if not isinstance(root_children, list): + root_children = [] + + if "GRID_ID" in root_children: + # Standard layout (ROOT -> GRID_ID): prepend banner ROW to GRID children. + _prepend_row_to_grid(position_json, row_key) + return position_json + + # Non-standard layout (e.g. ROOT -> TABS with GRID_ID orphaned): normalize + # the tree to ROOT -> GRID_ID -> [ROW-banner, ...] so the + # banner is guaranteed to render (matches the proven-working manual example). + _promote_grid_root(position_json, root, root_children, row_key) return position_json # #endregion Core.LayoutUtils.InsertBannerMarkdownAtTop +# #region Core.LayoutUtils.AddGridToParents [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Recursively ensure GRID_ID is present in the parent chain of a node and +# all of its descendants (used when moving a subtree under GRID_ID). +# @PRE position_json has node_key. node_key is not ROOT_ID. +# @POST position_json mutated; GRID_ID inserted into parents lists. +def _add_grid_to_parents(position_json: dict, node_key: str) -> None: + node = position_json.get(node_key) + if not isinstance(node, dict): + return + parents = node.get("parents") + if isinstance(parents, list) and "GRID_ID" not in parents: + if "ROOT_ID" in parents: + parents.insert(parents.index("ROOT_ID") + 1, "GRID_ID") + else: + parents.insert(0, "GRID_ID") + children = node.get("children") + if isinstance(children, list): + for child in children: + _add_grid_to_parents(position_json, child) +# #endregion Core.LayoutUtils.AddGridToParents + + +# #region Core.LayoutUtils.PromoteGridRoot [C:2] [TYPE Function] +# @ingroup Core +# @BRIEF Re-root a non-standard layout to ROOT -> GRID_ID and host the banner ROW +# as the first GRID child, moving the former root child(ren) under GRID. +# @PRE position_json has ROOT_ID and GRID_ID. root_children excludes GRID_ID. +# @POST position_json mutated; ROOT -> GRID_ID -> [row_key, former children...]. +def _promote_grid_root( + position_json: dict, root: dict, root_children: list, row_key: str +) -> None: + moved = [c for c in root_children if c != "GRID_ID"] + for child_key in moved: + _add_grid_to_parents(position_json, child_key) + + grid = position_json.get("GRID_ID") + existing_children: list = [] + grid_meta: dict | None = None + grid_parents: list | None = None + if isinstance(grid, dict): + ec = grid.get("children") + if isinstance(ec, list): + existing_children = list(ec) + if isinstance(grid.get("meta"), dict): + grid_meta = grid["meta"] + if isinstance(grid.get("parents"), list): + grid_parents = grid["parents"] + + new_children = [row_key, *moved] + for child_key in existing_children: + if child_key not in new_children and child_key != row_key: + new_children.append(child_key) + + grid_entry: dict = { + "type": "GRID", + "id": "GRID_ID", + "children": new_children, + "parents": grid_parents or ["ROOT_ID"], + } + if grid_meta is not None: + grid_entry["meta"] = grid_meta + + position_json["GRID_ID"] = grid_entry + root["children"] = ["GRID_ID"] +# #endregion Core.LayoutUtils.PromoteGridRoot + + # #region Core.LayoutUtils.UpdateBannerMarkdownContent [C:1] [TYPE Function] # @BRIEF Update the code content and adaptive height of an existing banner markdown element. # @PRE position_json has markdown_key. content is the new HTML/markdown string. @@ -155,44 +323,70 @@ def update_banner_markdown_content( # #endregion Core.LayoutUtils.UpdateBannerMarkdownContent +# #region Core.LayoutUtils.CaptureMarkdownY [C:1] [TYPE Function] +# @BRIEF Return the y-coordinate of a banner MARKDOWN node, or None. +# @POST Returns int y or None. +def _capture_markdown_y(position_json: dict, md_key: str) -> int | None: + data = position_json.get(md_key) + if not isinstance(data, dict): + return None + meta = data.get("meta", {}) + if not isinstance(meta, dict): + return None + return meta.get("y", 0) +# #endregion Core.LayoutUtils.CaptureMarkdownY + + +# #region Core.LayoutUtils.RemoveRowFromChildren [C:1] [TYPE Function] +# @BRIEF Remove a banner ROW key from a container's children list if present. +# @POST position_json mutated; row_key removed from container children. +def _remove_row_from_children( + position_json: dict, container_key: str, row_key: str +) -> None: + container = position_json.get(container_key) + if not isinstance(container, dict): + return + children = container.get("children", []) + if isinstance(children, list) and row_key in children: + children.remove(row_key) +# #endregion Core.LayoutUtils.RemoveRowFromChildren + + +# #region Core.LayoutUtils.ShiftUpAfterRemoval [C:1] [TYPE Function] +# @BRIEF Shift every positioned element with y > 0 up by an offset after the banner +# is removed from the top. +# @POST position_json mutated; y-coordinates decreased. +def _shift_up_after_removal(position_json: dict, offset: int = 2) -> None: + for _key, value in position_json.items(): + if isinstance(value, dict): + meta = value.get("meta", {}) + if isinstance(meta, dict): + cur_y = meta.get("y", 0) + if isinstance(cur_y, (int, float)) and cur_y > 0: + meta["y"] = cur_y - offset +# #endregion Core.LayoutUtils.ShiftUpAfterRemoval + + # #region Core.LayoutUtils.RemoveBannerFromPosition [C:2] [TYPE Function] # @ingroup Core -# @BRIEF Remove the banner ROW + MARKDOWN pair from the position dict and GRID children. -# Shifts remaining items up by 2 if the banner was at y=0. +# @BRIEF Remove the banner ROW + MARKDOWN pair from the position dict and its +# container children. Shifts remaining items up by 2 if the banner was at y=0. # @PRE position_json is a mutable dict. chart_id identifies the banner. # @POST position_json is mutated; ROW and MARKDOWN entries removed. def remove_banner_from_position(position_json: dict, chart_id: int) -> dict: row_key, md_key = _generate_banner_id(chart_id) - removed_y = None - # Remove MARKDOWN entry - if md_key in position_json: - markdown_data = position_json[md_key] - if isinstance(markdown_data, dict): - meta = markdown_data.get("meta", {}) - if isinstance(meta, dict): - removed_y = meta.get("y", 0) - del position_json[md_key] - - # Remove ROW entry + removed_y = _capture_markdown_y(position_json, md_key) + position_json.pop(md_key, None) position_json.pop(row_key, None) - # Remove ROW from GRID_ID children - grid = position_json.get("GRID_ID") - if grid and isinstance(grid, dict): - grid_children = grid.get("children", []) - if isinstance(grid_children, list) and row_key in grid_children: - grid_children.remove(row_key) + # Remove ROW from GRID_ID children and ROOT_ID children (covers both layouts) + _remove_row_from_children(position_json, "GRID_ID", row_key) + _remove_row_from_children(position_json, "ROOT_ID", row_key) # Shift remaining items up if removed_y is not None and removed_y == 0: - for key, value in position_json.items(): - if isinstance(value, dict): - meta = value.get("meta", {}) - if isinstance(meta, dict): - cur_y = meta.get("y", 0) - if isinstance(cur_y, (int, float)) and cur_y > 0: - meta["y"] = cur_y - 2 + _shift_up_after_removal(position_json) return position_json # #endregion Core.LayoutUtils.RemoveBannerFromPosition diff --git a/backend/src/services/maintenance/_banner_renderer.py b/backend/src/services/maintenance/_banner_renderer.py index 78977ae86..45949a3c5 100644 --- a/backend/src/services/maintenance/_banner_renderer.py +++ b/backend/src/services/maintenance/_banner_renderer.py @@ -166,12 +166,12 @@ def _build_banner_text_for_dashboard( # #region Services.BannerRenderer.RebuildBanner [C:3] [TYPE Function] [SEMANTICS maintenance, banner, rebuild, text, update] # @ingroup Services -# @BRIEF Rebuild aggregated banner text for a banner and update the Superset chart. +# @BRIEF Rebuild aggregated banner text for a banner and update the MARKDOWN element. # Query all active MaintenanceDashboardState records linked to banner, -# build aggregated text via build_banner_text, update markdown chart in Superset. -# @PRE banner_id points to an active MaintenanceDashboardBanner with a valid chart_id. -# @POST Banner chart in Superset updated with current aggregated text. DB banner_text updated. -# @SIDE_EFFECT Modifies Superset chart. Writes DB. +# build aggregated text via build_banner_text, update MARKDOWN in dashboard layout. +# @PRE banner_id points to an active MaintenanceDashboardBanner with a valid chart_id key. +# @POST MARKDOWN element updated with current aggregated text. DB banner_text updated. +# @SIDE_EFFECT Modifies dashboard layout. Writes DB. # @RELATION DEPENDS_ON -> [Services.BannerRenderer.BuildBannerText] # @RELATION DEPENDS_ON -> [Core.DashboardsWrite.SupersetDashboardsWriteMixin] async def rebuild_banner( diff --git a/backend/src/services/maintenance/_chart_manager.py b/backend/src/services/maintenance/_chart_manager.py index 3339a82f7..17d11dd72 100644 --- a/backend/src/services/maintenance/_chart_manager.py +++ b/backend/src/services/maintenance/_chart_manager.py @@ -1,17 +1,19 @@ # #region Services.ChartManager.MaintenanceChartManager [C:3] [TYPE Module] [SEMANTICS maintenance, chart, banner, lifecycle] # @defgroup Services Module group. -# @BRIEF Chart operations for maintenance banners: create/get banner charts, process -# per-dashboard state transitions for start and end orchestrators. +# @BRIEF Banner element operations for maintenance banners: ensure/remove native MARKDOWN +# banner elements, process per-dashboard state transitions for start and end orchestrators. # @LAYER Service # @RELATION DEPENDS_ON -> [Core.DashboardsWrite.SupersetDashboardsWriteMixin] # @RELATION DEPENDS_ON -> [Models.Maintenance.MaintenanceModels] from typing import Any +import uuid from sqlalchemy.orm import Session from ...core.logger import belief_scope, logger as app_logger from ...core.superset_client import SupersetClient +from ...core.superset_client._layout_utils import _is_key_reachable_from_root from ...models.maintenance import ( MaintenanceDashboardBanner, MaintenanceDashboardBannerStatus, @@ -25,12 +27,19 @@ from ._dashboard_scanner import _resolve_dashboard_title # #region Services.ChartManager.EnsureBannerChart [C:3] [TYPE Function] [SEMANTICS maintenance, banner, chart, creation] # @ingroup Services # @BRIEF Get or create a MaintenanceDashboardBanner for a dashboard in the target environment. -# If an active banner exists in DB, return it. Otherwise, create a markdown chart in Superset, -# update dashboard layout, and store the banner row. +# If an active banner exists in DB, return it. Otherwise, insert a native MARKDOWN banner +# into the dashboard layout and store the banner row. # @PRE dashboard_id exists in Superset. superset_client has write access. -# @POST Returns MaintenanceDashboardBanner with chart_id set. Chart is placed at top of dashboard. -# @SIDE_EFFECT Creates chart in Superset, modifies dashboard layout, writes DB row. +# @POST Returns MaintenanceDashboardBanner with chart_id set. Native MARKDOWN placed at top +# of dashboard layout. +# @SIDE_EFFECT Modifies dashboard layout, writes DB row. Does NOT create a chart resource. # @RELATION DEPENDS_ON -> [Core.DashboardsWrite.SupersetDashboardsWriteMixin] +# @RATIONALE No chart resource is created: the banner is a native MARKDOWN element in the +# dashboard layout; chart_id is a synthetic key used only for MARKDOWN-banner-{id} and +# ROW-banner-{id} layout keys and DB tracking. +# @REJECTED Creating a real markdown chart via create_markdown_chart was rejected — it +# produced orphan "Maintenance Banner" charts (e.g. chart 3696 in prod) that pollute the +# Charts menu and dashboard exports and are never rendered. async def ensure_banner_chart( dashboard_id: int, environment_id: str, @@ -42,7 +51,7 @@ async def ensure_banner_chart( f"dashboard_id={dashboard_id}", ): app_logger.reason( - "Ensuring banner chart exists", + "Ensuring banner element exists", extra={"dashboard_id": dashboard_id}, ) @@ -58,56 +67,56 @@ async def ensure_banner_chart( ) if existing: app_logger.reason( - "Found existing active banner, verifying chart in Superset", + "Found existing active banner, verifying MARKDOWN element in layout", extra={"banner_id": existing.id, "chart_id": existing.chart_id}, ) - # Verify the chart still exists in Superset + # Verify the native MARKDOWN element is still present in the RENDERED tree + # (reachable from ROOT_ID). Presence in the layout dict is not enough: + # dashboards corrupted by the old bug keep the element under an orphaned + # GRID_ID, where it never renders. try: - await superset_client.get_chart(existing.chart_id) - app_logger.reflect( - "Existing banner chart is alive, reusing", - extra={"banner_id": existing.id, "chart_id": existing.chart_id}, - ) - return existing - except Exception: + layout = await superset_client.get_dashboard_layout(existing.dashboard_id) + markdown_key = f"MARKDOWN-banner-{existing.chart_id}" + if _is_key_reachable_from_root(layout, markdown_key): + app_logger.reflect( + "Existing banner element is alive, reusing", + extra={"banner_id": existing.id, "chart_id": existing.chart_id}, + ) + return existing app_logger.explore( - "Existing banner chart is stale (not found in Superset), marking as REMOVED", + "Existing banner element not reachable from ROOT, marking as REMOVED", extra={ "banner_id": existing.id, "chart_id": existing.chart_id, }, + error="markdown key not reachable in dashboard layout", ) - existing.status = MaintenanceDashboardBannerStatus.REMOVED - db_session.flush() - # Fall through to create a new chart + except Exception as e: + app_logger.explore( + "Failed to verify existing banner, marking as REMOVED", + extra={ + "banner_id": existing.id, + "chart_id": existing.chart_id, + }, + error=str(e), + ) + existing.status = MaintenanceDashboardBannerStatus.REMOVED + db_session.flush() + # Fall through to create a new banner - # Create markdown chart in Superset - try: - chart_id = await superset_client.create_markdown_chart( - dashboard_id, "*Maintenance banner placeholder*" - ) - except Exception as e: - app_logger.explore( - "Failed to create markdown chart", - extra={"dashboard_id": dashboard_id}, - error=str(e), - ) - raise + # Generate a synthetic chart_id used only as a layout key for the native + # MARKDOWN element. No chart resource is created (would pollute Charts menu). + chart_id = int(uuid.uuid4().int % 2_000_000_000) - # Insert chart at top of dashboard layout + # Insert native MARKDOWN banner at top of dashboard layout try: await superset_client.update_dashboard_layout(dashboard_id, chart_id) except Exception as e: app_logger.explore( - "Failed to update dashboard layout, deleting chart", + "Failed to update dashboard layout", extra={"dashboard_id": dashboard_id, "chart_id": chart_id}, error=str(e), ) - # Clean up the chart we just created - try: - await superset_client.delete_chart(chart_id) - except Exception: - pass raise # Persist banner row @@ -122,7 +131,7 @@ async def ensure_banner_chart( db_session.flush() app_logger.reflect( - "Banner chart created and persisted", + "Banner element created and persisted", extra={ "banner_id": banner.id, "chart_id": chart_id, @@ -134,9 +143,9 @@ async def ensure_banner_chart( # #region Services.ChartManager.ProcessDashboardsForStart [C:3] [TYPE Function] -# @BRIEF Process each dashboard for a start maintenance event: ensure banner chart, -# create dashboard state, build banner text, update chart markdown. -# @SIDE_EFFECT Creates/updates Superset charts, writes DB rows. +# @BRIEF Process each dashboard for a start maintenance event: ensure banner element, +# create dashboard state, build banner text, update MARKDOWN content. +# @SIDE_EFFECT Modifies Superset dashboard layouts, writes DB rows. # @RELATION CALLS -> [Services.ChartManager.EnsureBannerChart] # @RELATION CALLS -> [Services.BannerRenderer.BuildBannerTextForDashboard] # @RELATION CALLS -> [Services.DashboardScanner.ResolveDashboardTitle] @@ -159,7 +168,7 @@ async def _process_dashboards_for_start( extra={"dashboard_id": dash_id, "event_id": event_id}, ) try: - # Ensure banner chart exists + # Ensure banner element exists banner = await ensure_banner_chart( dash_id, environment_id, @@ -238,8 +247,8 @@ async def _process_dashboards_for_start( # #region Services.ChartManager.ProcessStatesForEnd [C:3] [TYPE Function] # @BRIEF Process each dashboard state for an end maintenance event: if other events -# still use same banner → rebuild; if no other events → remove banner (chart + layout). -# @SIDE_EFFECT Modifies Superset dashboards (removes charts, updates layouts). Writes DB. +# still use same banner → rebuild; if no other events → remove banner (layout + row). +# @SIDE_EFFECT Modifies Superset dashboard layouts. Writes DB. # @RELATION CALLS -> [Services.BannerRenderer.RebuildBanner] async def _process_states_for_end( states: list[MaintenanceDashboardState], @@ -290,7 +299,7 @@ async def _process_states_for_end( state.status = MaintenanceDashboardStateStatus.REMOVING db_session.flush() - # Remove chart from layout + # Remove banner MARKDOWN element from layout if banner.chart_id: try: await superset_client.remove_chart_from_layout( @@ -298,7 +307,7 @@ async def _process_states_for_end( ) except Exception as e: app_logger.explore( - "Failed to remove chart from layout", + "Failed to remove banner from layout", extra={ "dashboard_id": dash_id, "chart_id": banner.chart_id, @@ -311,24 +320,6 @@ async def _process_states_for_end( ) continue - # Delete chart - try: - await superset_client.delete_chart(banner.chart_id) - except Exception as e: - app_logger.explore( - "Failed to delete chart", - extra={ - "chart_id": banner.chart_id, - "dashboard_id": dash_id, - }, - error=str(e), - ) - state.status = MaintenanceDashboardStateStatus.REMOVAL_FAILED - failed_removals.append( - {"id": dash_id, "title": str(dash_id), "error": str(e)} - ) - continue - # Mark banner as removed banner.status = MaintenanceDashboardBannerStatus.REMOVED state.status = MaintenanceDashboardStateStatus.REMOVED diff --git a/backend/tests/services/maintenance/test_chart_manager.py b/backend/tests/services/maintenance/test_chart_manager.py index 9e6e71a05..1de659ea9 100644 --- a/backend/tests/services/maintenance/test_chart_manager.py +++ b/backend/tests/services/maintenance/test_chart_manager.py @@ -2,15 +2,16 @@ # @BRIEF Tests for maintenance chart manager — ensure_banner_chart, process_dashboards, process_states. # @RELATION BINDS_TO -> [Services.ChartManager.MaintenanceChartManager] -import sys from pathlib import Path +import sys + sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / "src")) +import pytest from unittest.mock import AsyncMock, MagicMock, patch -import pytest - from src.models.maintenance import ( + DashboardScope, MaintenanceDashboardBanner, MaintenanceDashboardBannerStatus, MaintenanceDashboardState, @@ -18,7 +19,6 @@ from src.models.maintenance import ( MaintenanceEvent, MaintenanceEventStatus, MaintenanceSettings, - DashboardScope, ) @@ -34,32 +34,33 @@ def mock_db(): @pytest.fixture def mock_superset(): client = AsyncMock() - client.get_chart.return_value = {"id": 123, "result": {"viz_type": "markdown"}} - client.create_markdown_chart.return_value = 456 + client.get_dashboard_layout.return_value = { + "ROOT_ID": {"children": ["GRID_ID"]}, + "GRID_ID": {"children": []}, + } client.update_dashboard_layout.return_value = {"result": "ok"} client.update_banner_on_dashboard.return_value = {"result": "ok"} client.remove_chart_from_layout.return_value = {"result": "ok"} - client.delete_chart.return_value = {"result": "ok"} return client class TestEnsureBannerChart: @pytest.mark.asyncio async def test_creates_new_banner(self, mock_db, mock_superset): - """No existing banner → creates chart, updates layout, persists banner.""" + """No existing banner → inserts native MARKDOWN, persists banner, no chart.""" from src.services.maintenance._chart_manager import ensure_banner_chart result = await ensure_banner_chart(1, "env1", mock_superset, mock_db) - assert result.chart_id == 456 + assert isinstance(result.chart_id, int) assert result.status == MaintenanceDashboardBannerStatus.ACTIVE - mock_superset.create_markdown_chart.assert_called_once_with(1, "*Maintenance banner placeholder*") - mock_superset.update_dashboard_layout.assert_called_once_with(1, 456) + mock_superset.create_markdown_chart.assert_not_called() + mock_superset.update_dashboard_layout.assert_called_once_with(1, result.chart_id) mock_db.add.assert_called_once() mock_db.flush.assert_called() @pytest.mark.asyncio async def test_reuses_existing_banner(self, mock_db, mock_superset): - """Existing active banner → verifies chart in Superset and reuses.""" + """Existing active banner with MARKDOWN element in layout → reuses.""" from src.services.maintenance._chart_manager import ensure_banner_chart existing = MaintenanceDashboardBanner( @@ -67,16 +68,21 @@ class TestEnsureBannerChart: chart_id=123, status=MaintenanceDashboardBannerStatus.ACTIVE, banner_text="" ) mock_db.query.return_value.filter.return_value.first.return_value = existing - mock_superset.get_chart.return_value = {"id": 123} + mock_superset.get_dashboard_layout.return_value = { + "ROOT_ID": {"children": ["GRID_ID"]}, + "GRID_ID": {"children": ["ROW-banner-123"]}, + "ROW-banner-123": {"children": ["MARKDOWN-banner-123"]}, + "MARKDOWN-banner-123": {}, + } result = await ensure_banner_chart(1, "env1", mock_superset, mock_db) assert result is existing - mock_superset.get_chart.assert_called_once_with(123) - mock_superset.create_markdown_chart.assert_not_called() + mock_superset.get_dashboard_layout.assert_called_once_with(1) + mock_superset.update_dashboard_layout.assert_not_called() @pytest.mark.asyncio async def test_stale_banner_removed_and_recreated(self, mock_db, mock_superset): - """Existing but stale banner → marked REMOVED, new one created.""" + """Existing but stale (MARKDOWN missing from layout) → marked REMOVED, new one created.""" from src.services.maintenance._chart_manager import ensure_banner_chart existing = MaintenanceDashboardBanner( @@ -84,41 +90,27 @@ class TestEnsureBannerChart: chart_id=123, status=MaintenanceDashboardBannerStatus.ACTIVE, banner_text="" ) mock_db.query.return_value.filter.return_value.first.return_value = existing - mock_superset.get_chart.side_effect = Exception("Chart not found") + # Element exists in layout dict but is NOT reachable from ROOT (orphaned GRID) + mock_superset.get_dashboard_layout.return_value = { + "ROOT_ID": {"children": ["TABS-main"]}, + "GRID_ID": {"children": ["ROW-banner-123"]}, + "ROW-banner-123": {"children": ["MARKDOWN-banner-123"]}, + "MARKDOWN-banner-123": {}, + } result = await ensure_banner_chart(1, "env1", mock_superset, mock_db) assert existing.status == MaintenanceDashboardBannerStatus.REMOVED - assert result.chart_id == 456 # new chart - mock_superset.create_markdown_chart.assert_called_once() + assert isinstance(result.chart_id, int) + mock_superset.update_dashboard_layout.assert_called_once() @pytest.mark.asyncio - async def test_create_chart_failure(self, mock_db, mock_superset): - """Failure to create chart → exception propagates.""" - from src.services.maintenance._chart_manager import ensure_banner_chart - - mock_superset.create_markdown_chart.side_effect = Exception("API error") - with pytest.raises(Exception, match="API error"): - await ensure_banner_chart(1, "env1", mock_superset, mock_db) - - @pytest.mark.asyncio - async def test_layout_update_failure_cleans_up_chart(self, mock_db, mock_superset): - """Failure to update layout → deletes chart and raises.""" + async def test_layout_update_failure(self, mock_db, mock_superset): + """Failure to update layout → exception propagates.""" from src.services.maintenance._chart_manager import ensure_banner_chart mock_superset.update_dashboard_layout.side_effect = Exception("Layout error") with pytest.raises(Exception, match="Layout error"): await ensure_banner_chart(1, "env1", mock_superset, mock_db) - mock_superset.delete_chart.assert_called_once_with(456) - - @pytest.mark.asyncio - async def test_layout_update_failure_cleanup_ignores_delete_error(self, mock_db, mock_superset): - """Delete chart failure during cleanup does not mask original error.""" - from src.services.maintenance._chart_manager import ensure_banner_chart - - mock_superset.update_dashboard_layout.side_effect = Exception("Layout error") - mock_superset.delete_chart.side_effect = Exception("Delete also failed") - with pytest.raises(Exception, match="Layout error"): - await ensure_banner_chart(1, "env1", mock_superset, mock_db) class TestProcessDashboardsForStart: @@ -156,7 +148,7 @@ class TestProcessDashboardsForStart: """Exception outside banner creation → recorded as failed.""" from src.services.maintenance._chart_manager import _process_dashboards_for_start - mock_superset.create_markdown_chart.side_effect = Exception("Chart creation failed") + mock_superset.update_dashboard_layout.side_effect = Exception("Layout update failed") successful, failed, banners = await _process_dashboards_for_start( [1], "evt1", "env1", "Template", "UTC", mock_superset, mock_db @@ -223,7 +215,6 @@ class TestProcessStatesForEnd: assert banner.status == MaintenanceDashboardBannerStatus.REMOVED assert state.status == MaintenanceDashboardStateStatus.REMOVED mock_superset.remove_chart_from_layout.assert_called_once_with(1, 123) - mock_superset.delete_chart.assert_called_once_with(123) @pytest.mark.asyncio async def test_remove_chart_failure(self, mock_db, mock_superset): @@ -248,29 +239,6 @@ class TestProcessStatesForEnd: assert len(failed) == 1 assert state.status == MaintenanceDashboardStateStatus.REMOVAL_FAILED - @pytest.mark.asyncio - async def test_delete_chart_failure(self, mock_db, mock_superset): - """Failure to delete chart → REMOVAL_FAILED.""" - from src.services.maintenance._chart_manager import _process_states_for_end - - banner = MaintenanceDashboardBanner( - id="b1", dashboard_id=1, chart_id=123, - status=MaintenanceDashboardBannerStatus.ACTIVE, banner_text="old" - ) - state = MaintenanceDashboardState( - event_id="evt1", dashboard_id=1, banner_id="b1", - status=MaintenanceDashboardStateStatus.ACTIVE - ) - - mock_db.query.return_value.filter.return_value.filter.return_value.filter.return_value.count.return_value = 0 - mock_db.query.return_value.filter.return_value.first.return_value = banner - mock_superset.delete_chart.side_effect = Exception("Delete failed") - - removed, failed = await _process_states_for_end([state], "evt1", mock_superset, mock_db) - assert removed == 0 - assert len(failed) == 1 - assert state.status == MaintenanceDashboardStateStatus.REMOVAL_FAILED - @pytest.mark.asyncio async def test_no_banner_record(self, mock_db, mock_superset): """State with banner_id but no banner record → REMOVED.""" diff --git a/backend/tests/test_core/test_layout_utils.py b/backend/tests/test_core/test_layout_utils.py index cfcf992fa..e36c2d899 100644 --- a/backend/tests/test_core/test_layout_utils.py +++ b/backend/tests/test_core/test_layout_utils.py @@ -187,6 +187,80 @@ class TestInsertBannerMarkdownAtTop: assert row["type"] == "ROW" assert row["children"] == ["MARKDOWN-banner-7"] + def test_standard_root_grid_prepends_banner(self): + from src.core.superset_client._layout_utils import insert_banner_markdown_at_top + + position_json = { + "ROOT_ID": {"type": "ROOT", "children": ["GRID_ID"]}, + "GRID_ID": {"type": "GRID", "children": ["ROW-existing", "TABS-main"]}, + "ROW-existing": {"type": "ROW", "meta": {"y": 0}}, + "TABS-main": {"type": "TABS", "children": [], "parents": ["ROOT_ID", "GRID_ID"]}, + } + insert_banner_markdown_at_top(position_json, 10, "banner") + children = position_json["GRID_ID"]["children"] + assert children[0] == "ROW-banner-10" + assert "ROW-existing" in children + assert "TABS-main" in children + # ROOT still points at GRID_ID + assert position_json["ROOT_ID"]["children"] == ["GRID_ID"] + + def test_root_points_to_tabs_normalizes_to_grid(self): + from src.core.superset_client._layout_utils import insert_banner_markdown_at_top + + # 0085-style layout: ROOT -> TABS directly, GRID_ID present but orphaned. + position_json = { + "ROOT_ID": {"type": "ROOT", "children": ["TABS-main"]}, + "GRID_ID": {"type": "GRID", "children": [], "parents": ["ROOT_ID"]}, + "TABS-main": { + "type": "TABS", + "children": ["TAB-a"], + "parents": ["ROOT_ID"], + }, + "TAB-a": {"type": "TAB", "children": [], "parents": ["ROOT_ID", "TABS-main"]}, + } + insert_banner_markdown_at_top(position_json, 10, "banner") + + # ROOT now points at GRID_ID, banner is first child + assert position_json["ROOT_ID"]["children"] == ["GRID_ID"] + grid_children = position_json["GRID_ID"]["children"] + assert grid_children[0] == "ROW-banner-10" + assert "TABS-main" in grid_children + # Moved subtree got GRID_ID added to parent chains + assert position_json["TABS-main"]["parents"] == ["ROOT_ID", "GRID_ID"] + assert position_json["TAB-a"]["parents"] == ["ROOT_ID", "GRID_ID", "TABS-main"] + # Banner nodes reference GRID_ID + assert position_json["ROW-banner-10"]["parents"] == ["ROOT_ID", "GRID_ID"] + assert position_json["MARKDOWN-banner-10"]["parents"] == [ + "ROOT_ID", + "GRID_ID", + "ROW-banner-10", + ] + + def test_insert_cleans_stale_banner_keys(self): + from src.core.superset_client._layout_utils import insert_banner_markdown_at_top + + # Corrupted 0085-style layout: orphaned GRID holds a stale banner from an + # old (chart-based) run. Insert must remove it so only the new banner renders. + position_json = { + "ROOT_ID": {"type": "ROOT", "children": ["TABS-main"]}, + "GRID_ID": { + "type": "GRID", + "children": ["ROW-banner-3696"], + "parents": ["ROOT_ID"], + }, + "ROW-banner-3696": {"type": "ROW", "children": ["MARKDOWN-banner-3696"]}, + "MARKDOWN-banner-3696": {"type": "MARKDOWN", "meta": {"code": "old"}}, + "TABS-main": {"type": "TABS", "children": [], "parents": ["ROOT_ID"]}, + } + insert_banner_markdown_at_top(position_json, 10, "new banner") + + assert "ROW-banner-3696" not in position_json + assert "MARKDOWN-banner-3696" not in position_json + assert "ROW-banner-3696" not in position_json["GRID_ID"]["children"] + # Only the new banner remains + assert "ROW-banner-10" in position_json["GRID_ID"]["children"][:1] + assert "MARKDOWN-banner-10" in position_json + class TestUpdateBannerMarkdownContent: """update_banner_markdown_content: position_json + key + content -> mutated dict.""" @@ -271,4 +345,67 @@ class TestRemoveBannerFromPosition: result = remove_banner_from_position(position_json, 99) assert result is position_json # no crash assert "ROW-other" in result + + def test_removes_banner_from_normalized_grid_root(self): + from src.core.superset_client._layout_utils import remove_banner_from_position + + # Layout produced by insert on a ROOT->TABS dashboard (normalized to ROOT->GRID). + position_json = { + "ROOT_ID": {"type": "ROOT", "children": ["GRID_ID"]}, + "GRID_ID": {"type": "GRID", "children": ["ROW-banner-1", "TABS-main"]}, + "ROW-banner-1": {"type": "ROW", "meta": {"y": 0}}, + "MARKDOWN-banner-1": {"type": "MARKDOWN", "meta": {"y": 0}}, + "TABS-main": {"type": "TABS", "children": [], "parents": ["ROOT_ID", "GRID_ID"]}, + } + remove_banner_from_position(position_json, 1) + assert "ROW-banner-1" not in position_json + assert "MARKDOWN-banner-1" not in position_json + assert "ROW-banner-1" not in position_json["GRID_ID"]["children"] + assert "TABS-main" in position_json["GRID_ID"]["children"] + + +class TestIsKeyReachableFromRoot: + """_is_key_reachable_from_root: children-graph reachability from ROOT_ID.""" + + def test_reachable_via_grid(self): + from src.core.superset_client._layout_utils import _is_key_reachable_from_root + + layout = { + "ROOT_ID": {"children": ["GRID_ID"]}, + "GRID_ID": {"children": ["ROW-banner-9"]}, + "ROW-banner-9": {"children": ["MARKDOWN-banner-9"]}, + "MARKDOWN-banner-9": {}, + } + assert _is_key_reachable_from_root(layout, "MARKDOWN-banner-9") is True + + def test_orphaned_under_unreachable_grid_is_not_reachable(self): + from src.core.superset_client._layout_utils import _is_key_reachable_from_root + + # 0085 corrupted state: key present in dict but GRID_ID not in ROOT tree. + layout = { + "ROOT_ID": {"children": ["TABS-main"]}, + "GRID_ID": {"children": ["ROW-banner-9"]}, + "ROW-banner-9": {"children": ["MARKDOWN-banner-9"]}, + "MARKDOWN-banner-9": {}, + "TABS-main": {"children": []}, + } + assert _is_key_reachable_from_root(layout, "MARKDOWN-banner-9") is False + + def test_missing_root_falls_back_to_presence(self): + from src.core.superset_client._layout_utils import _is_key_reachable_from_root + + layout = {"MARKDOWN-banner-9": {}} + assert _is_key_reachable_from_root(layout, "MARKDOWN-banner-9") is True + assert _is_key_reachable_from_root(layout, "MARKDOWN-banner-8") is False + + def test_cycle_does_not_hang(self): + from src.core.superset_client._layout_utils import _is_key_reachable_from_root + + layout = { + "ROOT_ID": {"children": ["A"]}, + "A": {"children": ["B"]}, + "B": {"children": ["A"]}, + "MARKDOWN-banner-9": {}, + } + assert _is_key_reachable_from_root(layout, "MARKDOWN-banner-9") is False # #endregion Test.LayoutUtils diff --git a/backend/tests/test_maintenance_service.py b/backend/tests/test_maintenance_service.py index de6ccb466..843bc6ec6 100644 --- a/backend/tests/test_maintenance_service.py +++ b/backend/tests/test_maintenance_service.py @@ -45,12 +45,15 @@ def mock_superset(): client.get_datasets.return_value = (0, []) client.get_dashboards.return_value = (0, []) client.get_dataset_detail.return_value = {"linked_dashboards": []} - client.create_markdown_chart.return_value = 12345 - client.get_chart.return_value = {"id": 12345, "result": {"viz_type": "markdown"}} + client.get_dashboard_layout.return_value = { + "ROOT_ID": {"children": ["GRID_ID"]}, + "GRID_ID": {"children": ["ROW-banner-999"]}, + "ROW-banner-999": {"children": ["MARKDOWN-banner-999"]}, + "MARKDOWN-banner-999": {}, + } client.update_banner_on_dashboard.return_value = {"result": "ok"} client.update_dashboard_layout.return_value = {"result": "ok"} client.remove_chart_from_layout.return_value = {"result": "ok"} - client.delete_chart.return_value = {"result": "ok"} return client @@ -219,21 +222,22 @@ class TestEnsureBannerChart: """Tests for ensure_banner_chart.""" # #region Test.MaintenanceService.TestCreatesNewBanner [C:2] [TYPE Function] - # @BRIEF No existing banner — creates new chart and banner row. + # @BRIEF No existing banner — inserts native MARKDOWN element and banner row. @pytest.mark.asyncio async def test_creates_new_banner(self, mock_superset, db_session): banner = await ensure_banner_chart( 101, "test-env", mock_superset, db_session ) assert banner.dashboard_id == 101 - assert banner.chart_id == 12345 + assert isinstance(banner.chart_id, int) assert banner.status == MaintenanceDashboardBannerStatus.ACTIVE - mock_superset.create_markdown_chart.assert_called_once_with(101, "*Maintenance banner placeholder*") - mock_superset.update_dashboard_layout.assert_called_once_with(101, 12345) + # No chart resource is ever created + mock_superset.create_markdown_chart.assert_not_called() + mock_superset.update_dashboard_layout.assert_called_once_with(101, banner.chart_id) # #endregion Test.MaintenanceService.TestCreatesNewBanner # #region Test.MaintenanceService.TestReturnsExistingBanner [C:2] [TYPE Function] - # @BRIEF Existing active banner — returns it. + # @BRIEF Existing active banner with MARKDOWN element in layout — returns it. @pytest.mark.asyncio async def test_returns_existing_banner(self, mock_superset, db_session): existing = MaintenanceDashboardBanner( @@ -251,17 +255,18 @@ class TestEnsureBannerChart: ) assert banner.id == existing.id assert banner.chart_id == 999 - mock_superset.create_markdown_chart.assert_not_called() + mock_superset.get_dashboard_layout.assert_called_once_with(101) + mock_superset.update_dashboard_layout.assert_not_called() # #endregion Test.MaintenanceService.TestReturnsExistingBanner - # #region Test.MaintenanceService.TestChartCreationFailure [C:2] [TYPE Function] - # @BRIEF Chart creation failure propagates. + # #region Test.MaintenanceService.TestLayoutUpdateFailure [C:2] [TYPE Function] + # @BRIEF Layout update failure propagates. @pytest.mark.asyncio - async def test_chart_creation_failure(self, mock_superset, db_session): - mock_superset.create_markdown_chart.side_effect = Exception("Chart error") - with pytest.raises(Exception, match="Chart error"): + async def test_layout_update_failure(self, mock_superset, db_session): + mock_superset.update_dashboard_layout.side_effect = Exception("Layout error") + with pytest.raises(Exception, match="Layout error"): await ensure_banner_chart(101, "test-env", mock_superset, db_session) - # #endregion Test.MaintenanceService.TestChartCreationFailure + # #endregion Test.MaintenanceService.TestLayoutUpdateFailure # ── T020: build_banner_text tests ───────────────────────────── @@ -490,9 +495,8 @@ class TestEndMaintenance: assert result["maintenance_id"] == pending_event.id assert result["removed_from"] >= 1 - # Verify chart was deleted + # Verify MARKDOWN element was removed from layout mock_superset.remove_chart_from_layout.assert_called() - mock_superset.delete_chart.assert_called_with(12345) # Verify event status db_session.refresh(pending_event) @@ -550,8 +554,8 @@ class TestEndMaintenance: assert result["removed_from"] >= 1 - # Chart should NOT be deleted (event2 still active) - mock_superset.delete_chart.assert_not_called() + # No chart to delete (event2 still active) — only banner rebuilt + mock_superset.remove_chart_from_layout.assert_not_called() # But rebuild should be called mock_superset.update_banner_on_dashboard.assert_called() diff --git a/scripts/cleanup_maintenance_banner_charts.py b/scripts/cleanup_maintenance_banner_charts.py new file mode 100644 index 000000000..fb71bc088 --- /dev/null +++ b/scripts/cleanup_maintenance_banner_charts.py @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +""" +Cleanup of orphaned "Maintenance Banner" charts in Superset. + +Context (ADR: maintenance banner is a native MARKDOWN element, not a chart): +Before the fix, maintenance:start created a real markdown CHART resource +(slice_name="Maintenance Banner", viz_type="markdown") that was never rendered +— the visible banner is a native MARKDOWN element in the dashboard position_json. +The fix stops creating these charts, but the ones already created remain in the +Charts menu and in dashboard exports. This script finds and deletes them. + +Safety: +- DRY-RUN by default. Pass --yes to actually delete. +- Matches charts by slice_name == "Maintenance Banner" AND viz_type == "markdown" + AND params containing the known placeholder text, so legitimate charts are + never matched. +- Re-checks every candidate via GET /chart/{id} before deletion. +- The dashboard layouts are NOT touched: banners live as native MARKDOWN elements + in position_json and are unaffected by chart deletion. Stale maintenance_banners + DB rows self-heal on the next maintenance:start (reachability liveness check). + +Usage: + python scripts/cleanup_maintenance_banner_charts.py [--yes] \ + [--url https://superset.example.com] [--username admin] [--password ...] + # or via env: SUPERSET_URL / SUPERSET_USERNAME / SUPERSET_PASSWORD / SUPERSET_VERIFY_SSL + +Output: per-chart decision (DELETE/SKIP) + JSON summary on stdout. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "backend")) + +from src.core.config_models import Environment # noqa: E402 +from src.core.superset_client import SupersetClient # noqa: E402 + +BANNER_SLICE_NAME = "Maintenance Banner" +BANNER_PLACEHOLDER = "Maintenance banner placeholder" + +GREEN = "\033[32m" +RED = "\033[31m" +YELLOW = "\033[33m" +RESET = "\033[0m" + + +def _parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--url", default=os.environ.get("SUPERSET_URL", "")) + parser.add_argument("--username", default=os.environ.get("SUPERSET_USERNAME", "")) + parser.add_argument("--password", default=os.environ.get("SUPERSET_PASSWORD", "")) + parser.add_argument( + "--verify-ssl", + action="store_true", + default=os.environ.get("SUPERSET_VERIFY_SSL", "1") not in ("0", "false", "False"), + ) + parser.add_argument( + "--yes", + action="store_true", + help="Actually delete matching charts. Without it the script only reports.", + ) + return parser.parse_args() + + +def _looks_like_bogus_banner(chart: dict) -> bool: + """A candidate chart is bogus only if it is a markdown chart with the banner name. + + The placeholder check happens later on the full chart payload (GET /chart/{id}); + here we only pre-filter the list response columns. + """ + return ( + chart.get("slice_name") == BANNER_SLICE_NAME + and chart.get("viz_type") == "markdown" + ) + + +def _params_contain_placeholder(chart: dict) -> bool: + params = chart.get("params") + if not isinstance(params, str): + return False + try: + parsed = json.loads(params) + except json.JSONDecodeError: + return False + markdown = parsed.get("markdown", "") if isinstance(parsed, dict) else "" + return BANNER_PLACEHOLDER in str(markdown) + + +async def _find_candidates(client: SupersetClient) -> list[dict]: + """Fetch all charts named 'Maintenance Banner' (paginated).""" + candidates: list[dict] = [] + page = 0 + page_size = 100 + while True: + query = { + "columns": ["id", "slice_name", "viz_type"], + "filters": [{"col": "slice_name", "opr": "eq", "value": BANNER_SLICE_NAME}], + "page_size": page_size, + "page": page, + } + count, charts = await client.get_charts(query) + for chart in charts: + if _looks_like_bogus_banner(chart): + candidates.append(chart) + if page * page_size + len(charts) >= count or not charts: + break + page += 1 + return candidates + + +async def _delete_chart(client: SupersetClient, chart_id: int) -> None: + await client.request(method="DELETE", endpoint=f"/chart/{chart_id}") + + +async def run(args: argparse.Namespace, client: SupersetClient | None = None) -> int: + if client is None: + missing = [ + name + for name, val in (("url", args.url), ("username", args.username), ("password", args.password)) + if not val + ] + if missing: + print(f"{RED}Missing required config: {', '.join(missing)} (args or env){RESET}", file=sys.stderr) + return 2 + + env = Environment( + id="cleanup", + name="Cleanup", + url=args.url, + username=args.username, + password=args.password, + stage="PROD", + verify_ssl=args.verify_ssl, + ) + client = SupersetClient(env) + + print(f"{YELLOW}Scanning {args.url} for '{BANNER_SLICE_NAME}' markdown charts...{RESET}") + candidates = await _find_candidates(client) + if not candidates: + print(f"{GREEN}No 'Maintenance Banner' markdown charts found — nothing to do.{RESET}") + return 0 + + print(f"Found {len(candidates)} candidate chart(s). Verifying each via GET /chart/{{id}}...") + decisions: dict[str, list[dict]] = {"DELETE": [], "SKIP": []} + for cand in candidates: + chart_id = int(cand["id"]) + try: + full = await client.get_chart(chart_id) + except Exception as exc: # noqa: BLE001 - report and continue + decisions["SKIP"].append({"id": chart_id, "reason": f"get_chart failed: {exc}"}) + print(f" {YELLOW}SKIP {chart_id}: cannot fetch full payload ({exc}){RESET}") + continue + + if not _params_contain_placeholder(full): + decisions["SKIP"].append( + {"id": chart_id, "reason": "params do not contain the banner placeholder — not our chart"} + ) + print(f" {YELLOW}SKIP {chart_id}: params lack placeholder — not a bogus banner chart{RESET}") + continue + + decisions["DELETE"].append({"id": chart_id}) + print(f" {RED}DELETE {chart_id}{RESET} ({BANNER_SLICE_NAME}, markdown, placeholder present)") + + if not args.yes: + print( + f"\n{YELLOW}DRY-RUN: {len(decisions['DELETE'])} chart(s) would be deleted. " + f"Re-run with --yes to apply.{RESET}" + ) + else: + for item in decisions["DELETE"]: + try: + await _delete_chart(client, item["id"]) + print(f" {GREEN}deleted {item['id']}{RESET}") + except Exception as exc: # noqa: BLE001 - report and continue + item["error"] = str(exc) + print(f" {RED}FAILED to delete {item['id']}: {exc}{RESET}") + + summary = { + "url": args.url, + "dry_run": not args.yes, + "candidates_found": len(candidates), + "decisions": decisions, + } + print(f"\n{json.dumps(summary, ensure_ascii=False, indent=2)}") + return 0 + + +def main() -> int: + args = _parse_args() + try: + return asyncio.run(run(args)) + except KeyboardInterrupt: + return 130 + + +if __name__ == "__main__": + sys.exit(main())