-
Notifications
You must be signed in to change notification settings - Fork 0
Migrate DuckDB to ClickHouse #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9e850d1
270f551
3159f6f
9cc6bf8
cb8a4d6
5021ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| """Endpoints for saving, loading, and sharing dashboard layouts.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import uuid | ||
| from typing import Any, Dict, List | ||
|
|
||
| from fastapi import APIRouter, HTTPException, Query | ||
|
|
||
| from app.services.layout_repository import ( | ||
| DashboardLayout, | ||
| get_layout_repository, | ||
| ) | ||
| from app.utils.logger import setup_logger | ||
|
|
||
| logger = setup_logger(__name__) | ||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.post("/layouts") | ||
| async def create_layout(payload: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Save a new dashboard layout (or overwrite one by id).""" | ||
| repo = get_layout_repository() | ||
| layout_id = payload.get("id") or str(uuid.uuid4()) | ||
| layout = DashboardLayout( | ||
| id=layout_id, | ||
| name=payload.get("name", "Untitled layout"), | ||
| owner=payload.get("owner", "anonymous"), | ||
| widgets=payload.get("widgets", []), | ||
| is_public=bool(payload.get("is_public", False)), | ||
| ) | ||
| saved = repo.save(layout) | ||
| return saved.to_dict() | ||
|
|
||
|
|
||
| @router.get("/layouts/{layout_id}") | ||
| async def get_layout(layout_id: str) -> Dict[str, Any]: | ||
| repo = get_layout_repository() | ||
| layout = repo.get(layout_id) | ||
| if not layout: | ||
| raise HTTPException(status_code=404, detail="Layout not found") | ||
|
Comment on lines
+36
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: backend/app/routers/layouts.py
Line: 36-41
Comment:
**Private Layouts Read By ID**
`get_layout` returns any stored layout by ID without checking ownership or `is_public`. With the new persistent repository, anyone who obtains a layout ID can read private dashboard widget data through this route.
How can I resolve this? If you propose a fix, please make it concise. |
||
| return layout.to_dict() | ||
|
|
||
|
|
||
| @router.get("/layouts") | ||
| async def list_layouts( | ||
| owner: str = Query("anonymous"), public: bool = Query(False) | ||
| ) -> Dict[str, Any]: | ||
| repo = get_layout_repository() | ||
| layouts: List[DashboardLayout] = ( | ||
| repo.list_public() if public else repo.list_by_owner(owner) | ||
| ) | ||
| return {"layouts": [l.to_dict() for l in layouts], "count": len(layouts)} | ||
|
|
||
|
|
||
| @router.delete("/layouts/{layout_id}") | ||
| async def delete_layout(layout_id: str) -> Dict[str, Any]: | ||
| repo = get_layout_repository() | ||
| ok = repo.delete(layout_id) | ||
| if not ok: | ||
| raise HTTPException(status_code=404, detail="Layout not found") | ||
|
Comment on lines
+56
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: backend/app/routers/layouts.py
Line: 56-61
Comment:
**Layouts Deleted By ID**
`delete_layout` deletes by layout ID alone, and the repository delete call has no owner predicate. Any caller with a saved layout ID can delete another user's dashboard layout.
How can I resolve this? If you propose a fix, please make it concise. |
||
| return {"deleted": layout_id} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| """OLAP analytics endpoints backed by the pluggable storage tier.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from fastapi import APIRouter, HTTPException, Query | ||
| from typing import Any, Dict, List | ||
|
|
||
| from app.services.olap.factory import get_olap_engine | ||
| from app.utils.logger import setup_logger | ||
|
|
||
| logger = setup_logger(__name__) | ||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.get("/olap/engine") | ||
| async def olap_engine() -> Dict[str, Any]: | ||
| """Report which OLAP backend is currently active.""" | ||
| engine = get_olap_engine() | ||
| return {"backend": engine.backend, "healthy": engine.health()} | ||
|
|
||
|
|
||
| @router.get("/olap/query") | ||
| async def olap_query( | ||
| sql: str = Query(..., description="Read-only SQL against the analytics tier"), | ||
| limit: int = Query(100, ge=1, le=1000), | ||
| ) -> Dict[str, Any]: | ||
| """Run a read-only analytics query against the active OLAP backend. | ||
|
|
||
| This is the single entry point used by the GenAI agent and the dashboard so | ||
| that, regardless of whether the tier is DuckDB, ClickHouse, or MotherDuck, | ||
| the rest of the app only depends on one interface. | ||
| """ | ||
| engine = get_olap_engine() | ||
| try: | ||
| safe_sql = sql.strip().rstrip(";") | ||
| if not safe_sql.lower().startswith(("select", "show", "with")): | ||
| raise HTTPException( | ||
| status_code=400, detail="Only read-only statements are permitted" | ||
| ) | ||
| rows = engine.query(f"{safe_sql} LIMIT {limit}") | ||
|
Comment on lines
+35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security and correctness issue in
Comment on lines
+35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The read-only check only verifies that the SQL starts with Prompt To Fix With AIThis is a comment left during a code review.
Path: backend/app/routers/olap.py
Line: 35-40
Comment:
**Prefix Check Allows Writes**
The read-only check only verifies that the SQL starts with `select`, `show`, or `with`, then forwards the full string to the engine. A request such as `select 1; drop table telemetry` passes the guard and can execute the later mutating statement before the appended limit helps.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: backend/app/routers/olap.py
Line: 40
Comment:
**Duplicate Limit Breaks Queries**
`olap_query` appends `LIMIT` even when the supplied SQL already has one. SQL produced by `/api/semantic/compile` with a `limit` becomes `... LIMIT n LIMIT m`, which DuckDB and ClickHouse reject and the API returns as a backend error.
How can I resolve this? If you propose a fix, please make it concise. |
||
| return {"backend": engine.backend, "rows": rows, "count": len(rows)} | ||
| except HTTPException: | ||
| raise | ||
| except Exception as exc: # pragma: no cover - backend-specific failures | ||
| logger.warning(f"OLAP query failed: {exc}") | ||
| raise HTTPException(status_code=502, detail=f"OLAP query failed: {exc}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| """Semantic layer endpoints: token-efficient, RBAC-safe prompt context.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from fastapi import APIRouter, HTTPException, Query | ||
| from typing import Dict, Any | ||
|
|
||
| from app.services.semantic_model import Dialect, get_semantic_model | ||
| from app.utils.logger import setup_logger | ||
|
|
||
| logger = setup_logger(__name__) | ||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.get("/semantic/context") | ||
| async def semantic_context( | ||
| role: str = Query("analyst", description="RBAC role requesting context"), | ||
| dialect: Dialect = Query(Dialect.DUCKDB, description="Target OLAP dialect"), | ||
| ) -> Dict[str, Any]: | ||
| """Return the compact semantic context for a role. | ||
|
|
||
| This structured description (not the raw schema) is what gets injected into | ||
| the Gemini prompt, shrinking token usage and constraining the model to the | ||
| role's entitlements. | ||
| """ | ||
| model = get_semantic_model() | ||
| try: | ||
| context = model.build_prompt_context(role, dialect) | ||
| except Exception as exc: | ||
| raise HTTPException(status_code=404, detail=str(exc)) | ||
| return {"role": role, "dialect": dialect.value, "context": context} | ||
|
|
||
|
|
||
| @router.post("/semantic/compile") | ||
| async def semantic_compile(payload: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Compile an authorized semantic query to engine-specific SQL. | ||
|
|
||
| Body: ``{role, measures, dimensions, filters?, order_by?, limit?, dialect?}``. | ||
| Raises 403-style 400 when the role is not entitled to part of the request. | ||
| """ | ||
| model = get_semantic_model() | ||
| try: | ||
| sql = model.compile( | ||
| role=payload["role"], | ||
| measures=payload.get("measures", []), | ||
| dimensions=payload.get("dimensions", []), | ||
| filters=payload.get("filters", []), | ||
| order_by=payload.get("order_by"), | ||
| order_direction=payload.get("order_direction", "DESC"), | ||
| limit=payload.get("limit"), | ||
| dialect=Dialect(payload.get("dialect", "duckdb")), | ||
| ) | ||
| except KeyError: | ||
| raise HTTPException(status_code=400, detail="Missing 'role' in body") | ||
| except Exception as exc: | ||
| raise HTTPException(status_code=400, detail=str(exc)) | ||
| return {"sql": sql} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create_layouttrustsownerandidfrom the request body, thensave()overwrites on ID conflicts. A caller can submit another user's layout ID and owner string to replace that saved dashboard's widgets or public flag.Prompt To Fix With AI