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.
This commit is contained in:
@@ -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, <former root child>...] 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
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
203
scripts/cleanup_maintenance_banner_charts.py
Normal file
203
scripts/cleanup_maintenance_banner_charts.py
Normal file
@@ -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())
|
||||
Reference in New Issue
Block a user