Spec: 5 user stories (P1-P3), 17 FRs, 7 SCs, 12 edge cases, RBAC matrix Plan: 7 research decisions, 15 semantic contracts (C1-C4) Data: 4 SQLAlchemy models (Banner entity enforces one-chart-per-dashboard) UX: async-only API, banner template with optional variables, admin settings UI Tasks: 64 tasks across 8 phases, 12 reused frontend components Workflow: component reuse scan mandated in speckit.plan/tasks + semantics-svelte Constitution: filled with 7 architecture principles from ADRs
174 lines
11 KiB
Markdown
174 lines
11 KiB
Markdown
# Research: Maintenance Banner for Dashboards
|
|
|
|
**Feature Branch**: `031-maintenance-banner`
|
|
**Created**: 2026-05-21
|
|
**Status**: Phase 0 Complete
|
|
|
|
## R1: Superset Chart Placement Mechanism
|
|
|
|
### Decision
|
|
Use **direct chart creation via `POST /api/v1/chart/` + layout manipulation via Superset's existing dashboard JSON metadata patch**, rather than the export→modify YAML→re-import cycle.
|
|
|
|
### Rationale
|
|
- The export-import cycle is designed for full dashboard migration (ADR-0004). For adding/removing a single chart, it's excessively heavy: exports a multi-MB ZIP, modifies YAML, re-uploads. For 50 dashboards this could take minutes.
|
|
- The `POST /chart/` endpoint is proven to work in the codebase (`seed_superset_load_test.py`, line 350). Creating a markdown chart (`viz_type: "markdown"`) with `dashboards: [dashboard_id]` links the chart to the dashboard.
|
|
- For positioning at the top, we need to modify the dashboard's `position_json` to insert the new chart at (0,0) with full width. This can be done via `PUT /api/v1/dashboard/{id}` with updated `position_json` and `json_metadata`.
|
|
- Superset REST API supports `PUT /dashboard/{id}` for updating dashboard metadata. We will extend `SupersetClient` with a `create_chart()` and `update_dashboard_layout()` method.
|
|
|
|
### Alternatives Considered
|
|
- **Export-import cycle**: Rejected — too slow for 50+ dashboards; each dashboard requires ZIP round-trip.
|
|
- **Superset's import API with selective overwrite**: Rejected — not granular enough; would replace the entire dashboard definition.
|
|
- **SQL-based direct manipulation of Superset's metadata DB**: Rejected — violates ADR-0003 (ss-tools is an external orchestrator, not integrated into Superset).
|
|
|
|
### Impact On Contracts / Tasks
|
|
- Adds `SupersetDashboardsWriteMixin` with `create_chart()` and `update_dashboard()` methods to `SupersetClient`.
|
|
- Chart removal pattern: store `chart_id` in `MaintenanceDashboardState`, delete via Superset chart delete API.
|
|
- Tasks: T0XX (SupersetClient extension), T0XX (chart create/delete service).
|
|
|
|
---
|
|
|
|
## R2: Module Placement
|
|
|
|
### Decision
|
|
Follow ADR-0001 canonical layout with these specific placements:
|
|
|
|
| Module | Path | Layer |
|
|
|--------|------|-------|
|
|
| API routes | `backend/src/api/routes/maintenance/` (package) | Route handler (C3) |
|
|
| API schemas | `backend/src/api/routes/maintenance/_schemas.py` | Pydantic models (C1) |
|
|
| Core service | `backend/src/services/maintenance_service.py` | Business logic (C4) |
|
|
| Superset client mixin | `backend/src/core/superset_client/_dashboards_write.py` | Superset API (C4) |
|
|
| SQL parser | `backend/src/services/sql_table_extractor.py` | Utility (C2/C3) |
|
|
| ORM models | `backend/src/models/maintenance.py` | SQLAlchemy (C1) |
|
|
| Frontend page | `frontend/src/routes/maintenance/+page.svelte` | SvelteKit page (C3) |
|
|
| Frontend components | `frontend/src/lib/components/MaintenanceBanner*.svelte` | Svelte 5 components (C3) |
|
|
| Frontend API client | `frontend/src/lib/api/maintenance.js` | API wrapper (C2) |
|
|
| Frontend store | `frontend/src/lib/stores/maintenance.svelte.js` | Svelte 5 rune store (C3) |
|
|
|
|
### Rationale
|
|
- Pattern B (package-based routes) for maintenance — mimics `dashboards/` organization with `_router.py`, `_routes.py`, `_schemas.py`. Single responsibility per file.
|
|
- Business logic in `services/` not `core/` because maintenance service is stateless and request-scoped (per ADR-0001 boundary rule #3).
|
|
- Superset API extension goes in `core/superset_client/` because it's a singleton client mixin (boundary rule #2).
|
|
|
|
### Alternatives Considered
|
|
- **Plugin-based (like migration.py)**: Rejected — maintenance banner is not a user-configurable plugin with its own lifecycle; it's a core feature with admin UI.
|
|
- **All logic in route handler**: Rejected — violates 400 LOC limit and C3 complexity for routes.
|
|
|
|
### Impact On Contracts / Tasks
|
|
- All task file paths must map to these locations.
|
|
- Contract IDs must reference these canonical paths.
|
|
|
|
---
|
|
|
|
## R3: SQL Table Name Extraction for Virtual Datasets
|
|
|
|
### Decision
|
|
**Extract `schema.table` patterns from raw SQL+Jinja text using three-phase approach: (1) detect Jinja block spans (`{% %}` and `{{ }}`), (2) in Jinja spans — extract table names from string literals (`"schema.table"`), (3) in SQL spans — regex global extraction + sqlparse filtering to reject string literal false positives.** Jinja is NOT pre-stripped — table names inside `{% set %}` blocks are preserved.
|
|
|
|
### Rationale
|
|
- Real-world Superset virtual datasets embed table names inside Jinja `{% set %}` blocks (e.g., a `selected_dataset` dictionary). If Jinja is stripped before parsing, these are lost.
|
|
- sqlparse cannot correctly tokenize Jinja code — treating `{% %}` as unknown tokens and potentially misclassifying positions. Therefore Jinja spans are detected first and processed separately.
|
|
- **Three-phase approach:**
|
|
1. **Span detection**: Split raw text into Jinja block spans and SQL spans using regex for `{%...%}` and `{{...}}` boundaries.
|
|
2. **Jinja spans**: Inside Jinja blocks, extract `"schema.table"` from string literal values — these are NOT SQL strings and must not be filtered by sqlparse.
|
|
3. **SQL spans**: Regex `[a-zA-Z][\w]*\.[a-zA-Z][\w]*` finds candidates; sqlparse tokenizer rejects any whose byte position falls inside a SQL string literal.
|
|
- This captures tables from ALL contexts — FROM, JOIN, Jinja `{% set %}` values — while filtering out `'2026.04.30'` (date in SQL string literal).
|
|
|
|
### Alternatives Considered
|
|
- **Strip Jinja → parse SQL (original plan)**: Rejected — loses tables embedded in Jinja `{% set %}` blocks (empirically proven with production dataset example).
|
|
- **sqlglot**: Rejected — would also miss Jinja-embedded tables (same fundamental problem); its AST capabilities are overkill when the extraction strategy is pattern-based, not structure-based.
|
|
- **Pure regex without sqlparse filter**: Rejected — false positives on date literals in string constants (`'2026.04.30'`).
|
|
|
|
### Impact On Contracts / Tasks
|
|
- `sql_table_extractor.py` module: C2 — two pure functions:
|
|
- `extract_table_candidates(raw_sql: str) -> list[str]` — regex global extraction
|
|
- `filter_string_literals(raw_sql: str, candidates: list[str]) -> set[str]` — sqlparse-based filtering
|
|
- No Jinja pre-stripping step needed (simpler, safer).
|
|
- Test corpus: production virtual dataset SQL+Jinja samples, date literals, string literals.
|
|
|
|
---
|
|
|
|
## R4: Async Task Orchestration
|
|
|
|
### Decision
|
|
Use the **existing `TaskManager`** with a **new task type `maintenance_banner_apply` / `maintenance_banner_remove`**. The `POST /api/maintenance/start` endpoint creates a MaintenanceEvent and dispatches a task via `task_manager.create_task()`.
|
|
|
|
### Rationale
|
|
- All existing long-running operations in ss-tools use TaskManager (migration, backup, LLM validation). Consistency reduces cognitive load and reuses scheduling/persistence/retry/WebSocket infrastructure.
|
|
- WebSocket-based progress updates are already wired in `app.py` for all task log queues.
|
|
- The operator gets a `task_id` immediately (per async clarification Q2) and can poll `GET /api/tasks/{task_id}` or subscribe to WebSocket events.
|
|
- Task status lifecycle: `PENDING → RUNNING → SUCCESS/FAILED` maps directly to `MaintenanceEvent.status: active/completed/failed`.
|
|
|
|
### Alternatives Considered
|
|
- **Celery / Redis Queue**: Rejected — ss-tools uses in-process APScheduler + async FastAPI; Celery adds Redis dependency and operational complexity.
|
|
- **Direct synchronous HTTP response after completion**: Rejected — fails on 100+ dashboards (HTTP timeout).
|
|
|
|
### Impact On Contracts / Tasks
|
|
- `MaintenanceEvent.task_id` links to `Task.id`.
|
|
- Task payload: `{"event_id": "m-...", "tables": [...], "action": "apply"|"remove"}`.
|
|
- No new task infrastructure needed — reuse `TaskManager.create_task()`.
|
|
|
|
---
|
|
|
|
## R5: Database Schema — Dedicated Tables vs AppConfigRecord
|
|
|
|
### Decision
|
|
Use **dedicated SQLAlchemy models** (`MaintenanceEvent`, `MaintenanceDashboardState`, `MaintenanceSettings`) with proper foreign keys and indexes rather than JSON blobs in `AppConfigRecord`.
|
|
|
|
### Rationale
|
|
- Maintenance events require relational queries: "which dashboards are affected by active events?", "find all events for dashboard D", "count active events". JSON in AppConfigRecord would require full-table-scan + client-side filtering.
|
|
- `MaintenanceDashboardState` has a foreign key to both `MaintenanceEvent` and acts as a junction table for the many-to-many relationship between events and dashboards.
|
|
- `MaintenanceSettings` is a single-row table (one settings record) — simpler than `AppConfigRecord` key-value pattern and provides typed columns with validation.
|
|
|
|
### Alternatives Considered
|
|
- **AppConfigRecord with JSON payload**: Rejected — no queryability, no foreign key constraints, no type safety.
|
|
- **Superset metadata DB modification**: Rejected — violates ADR-0003 (external orchestrator).
|
|
|
|
### Impact On Contracts / Tasks
|
|
- Three new SQLAlchemy models in `backend/src/models/maintenance.py`.
|
|
- Alembic migration required.
|
|
- Index on `MaintenanceDashboardState(dashboard_id, status)` for dashboard-hub queries.
|
|
|
|
---
|
|
|
|
## R6: Frontend Architecture — Svelte 5 Runes
|
|
|
|
### Decision
|
|
New SvelteKit route `/maintenance` with three Svelte 5 runes-based components: `MaintenanceBannerManagement.svelte` (settings + events table), maintenance badge integration into existing `DashboardGrid` component, and API client module `maintenance.js`.
|
|
|
|
### Rationale
|
|
- Svelte 5 runes (`$state`, `$derived`, `$effect`) mandated by ADR-0006. No legacy `$:` or `writable` stores.
|
|
- `fromStore` + multiple `$derived` is explicitly REJECTED per ADR-0007 — use `$effect(() => store.subscribe(...))` pattern.
|
|
- State topology: page-level `$state` for events/settings, `$derived` for filtered views, `$effect` for 30s polling.
|
|
- API client follows existing pattern in `frontend/src/lib/api/` using `requestApi` wrapper.
|
|
|
|
### Alternatives Considered
|
|
- **SSR with SvelteKit load functions**: Rejected — ADR-0006 mandates SPA mode (adapter-static). All data fetched client-side via REST API.
|
|
- **Shared store for maintenance state**: The maintenance badge in Dashboard Hub needs data from maintenance events. Use a shared `maintenance.svelte.js` rune store.
|
|
|
|
### Impact On Contracts / Tasks
|
|
- New store: `frontend/src/lib/stores/maintenance.svelte.js` (C3).
|
|
- New route: `frontend/src/routes/maintenance/+page.svelte` (C3).
|
|
- Modified component: `frontend/src/routes/dashboards/+page.svelte` (add maintenance badge column/cell).
|
|
|
|
---
|
|
|
|
## R7: Authentication & Authorization
|
|
|
|
### Decision
|
|
Reuse existing RBAC system (ADR-0005). API endpoints require `operator` or `admin` role. UI management page requires `admin` role. Maintenance badge in Dashboard Hub is visible to all authenticated users.
|
|
|
|
### Rationale
|
|
- FR-014 mandates `operator` or `admin` for dashboard-modifying operations.
|
|
- UI settings page (scope, excluded/forced dashboards) is an administrative function — `admin` only.
|
|
- The badge in Dashboard Hub is read-only information, safe for all roles.
|
|
|
|
### Alternatives Considered
|
|
- **Separate `maintenance_admin` role**: Rejected — overcomplicates RBAC for a single feature.
|
|
- **Open API (no auth)**: Rejected — would allow unauthorized banner injection on production dashboards.
|
|
|
|
### Impact On Contracts / Tasks
|
|
- `POST/PUT /api/maintenance/*` endpoints use `Depends(require_role("operator"))`.
|
|
- `GET /api/maintenance/settings` uses `Depends(require_role("admin"))`.
|
|
- Badge data endpoint open to authenticated users.
|