refactor(connections): remove ConnectionConfig, migrate mapper to SQL Lab
Backend: - Remove ConnectionConfig model, CRUD routes (connections.py), and tests - Remove ensure_connection_configs_table from database.py - Migrate MapperPlugin from psycopg2 direct PostgreSQL to SupersetSqlLabExecutor - Add DatasetMapper.get_sqllab_mappings() with default information_schema query - Update MapColumnsRequest: connection_id → database_id + sql_query - Fix pydantic_settings v2 deprecation (Field(env=...) → validation_alias) - Clean up app.py, routes/__init__.py, database.py imports Frontend: - Remove DatabaseConnectionsTab, ConnectionForm, ConnectionList components - Remove /settings/connections route page and Navbar link - Remove connectionService.js, connections i18n files - Update MapperTool.svelte: postgres → sqllab source - Update Map Columns modal: database selector + SQL query instead of connection_id - Clean up settings page tabs, nav links, test mocks - Clean up i18n keys (settings, nav, mapper) Tests: 25/25 pass (14 030-feature + 11 route tests) Build: succeeds
This commit is contained in:
@@ -12,7 +12,6 @@
|
||||
# @RELATION DEPENDS_ON -> [PluginsRouter]
|
||||
# @RELATION DEPENDS_ON -> [TasksRouter]
|
||||
# @RELATION DEPENDS_ON -> [SettingsRouter]
|
||||
# @RELATION DEPENDS_ON -> [ConnectionsRouter]
|
||||
# @RELATION DEPENDS_ON -> [ReportsRouter]
|
||||
# @RELATION DEPENDS_ON -> [LlmRoutes]
|
||||
# @SIDE_EFFECT: Registers route group imports via __getattr__
|
||||
@@ -22,7 +21,6 @@ __all__ = [
|
||||
"assistant",
|
||||
"clean_release",
|
||||
"clean_release_v2",
|
||||
"connections",
|
||||
"dashboards",
|
||||
"dataset_review",
|
||||
"datasets",
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
# #region RoutesTestsConftest [TYPE Module] [C:1] [SEMANTICS test, fixture, mock, conftest]
|
||||
# @BRIEF Shared low-fidelity test doubles for API route test modules.
|
||||
|
||||
# Set required env vars before any app imports
|
||||
import os
|
||||
os.environ.setdefault("SECRET_KEY", "test-secret-key-for-testing")
|
||||
os.environ.setdefault("AUTH_DATABASE_URL", "sqlite:///:memory:test_auth")
|
||||
os.environ.setdefault("DEV_MODE", "true")
|
||||
class FakeQuery:
|
||||
"""Shared chainable query stub for route tests.
|
||||
WARNING: filter() is predicate-blind — all ownership and permission filters are
|
||||
|
||||
@@ -1,69 +0,0 @@
|
||||
# #region ConnectionsRoutesTests [TYPE Module] [C:3] [SEMANTICS test, connection, route, crud, bootstrap]
|
||||
# @BRIEF Verifies connection routes bootstrap their table before CRUD access.
|
||||
# @LAYER: API
|
||||
# @RELATION DEPENDS_ON -> ConnectionsRouter
|
||||
import asyncio
|
||||
import os
|
||||
from pathlib import Path
|
||||
import pytest
|
||||
import sys
|
||||
|
||||
from sqlalchemy import create_engine, inspect
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
from sqlalchemy.pool import StaticPool
|
||||
|
||||
# Force SQLite in-memory for database module imports.
|
||||
# @SIDE_EFFECT_WARNING: os.environ mutation at module import time — no teardown. This bleeds into all subsequently collected tests. Migrate to pytest.fixture(autouse=True) with monkeypatch.setenv.
|
||||
os.environ["DATABASE_URL"] = "sqlite:///:memory:"
|
||||
# @SIDE_EFFECT_WARNING: os.environ mutation at module import time — no teardown. This bleeds into all subsequently collected tests. Migrate to pytest.fixture(autouse=True) with monkeypatch.setenv.
|
||||
os.environ["TASKS_DATABASE_URL"] = "sqlite:///:memory:"
|
||||
# @SIDE_EFFECT_WARNING: os.environ mutation at module import time — no teardown. This bleeds into all subsequently collected tests. Migrate to pytest.fixture(autouse=True) with monkeypatch.setenv.
|
||||
os.environ["AUTH_DATABASE_URL"] = "sqlite:///:memory:"
|
||||
# @SIDE_EFFECT_WARNING: os.environ mutation at module import time — no teardown. This bleeds into all subsequently collected tests. Migrate to pytest.fixture(autouse=True) with monkeypatch.setenv.
|
||||
os.environ["ENVIRONMENT"] = "testing"
|
||||
backend_dir = str(Path(__file__).parent.parent.parent.parent.resolve())
|
||||
if backend_dir not in sys.path:
|
||||
sys.path.insert(0, backend_dir)
|
||||
@pytest.fixture
|
||||
def db_session():
|
||||
engine = create_engine(
|
||||
"sqlite:///:memory:",
|
||||
connect_args={"check_same_thread": False},
|
||||
poolclass=StaticPool,
|
||||
)
|
||||
session = sessionmaker(bind=engine)()
|
||||
try:
|
||||
yield session
|
||||
finally:
|
||||
session.close()
|
||||
# #region test_list_connections_bootstraps_missing_table [TYPE Function]
|
||||
# @RELATION BINDS_TO -> ConnectionsRoutesTests
|
||||
# @BRIEF Ensure listing connections auto-creates missing table and returns empty payload.
|
||||
def test_list_connections_bootstraps_missing_table(db_session):
|
||||
from src.api.routes.connections import list_connections
|
||||
result = asyncio.run(list_connections(db=db_session))
|
||||
inspector = inspect(db_session.get_bind())
|
||||
assert result == []
|
||||
assert "connection_configs" in inspector.get_table_names()
|
||||
# #endregion test_list_connections_bootstraps_missing_table
|
||||
# #region test_create_connection_bootstraps_missing_table [TYPE Function]
|
||||
# @RELATION BINDS_TO -> ConnectionsRoutesTests
|
||||
# @BRIEF Ensure connection creation bootstraps table and persists returned connection fields.
|
||||
def test_create_connection_bootstraps_missing_table(db_session):
|
||||
from src.api.routes.connections import ConnectionCreate, create_connection
|
||||
payload = ConnectionCreate(
|
||||
name="Analytics Warehouse",
|
||||
type="postgres",
|
||||
host="warehouse.internal",
|
||||
port=5432,
|
||||
database="analytics",
|
||||
username="reporter",
|
||||
password="secret",
|
||||
)
|
||||
created = asyncio.run(create_connection(connection=payload, db=db_session))
|
||||
inspector = inspect(db_session.get_bind())
|
||||
assert created.name == "Analytics Warehouse"
|
||||
assert created.host == "warehouse.internal"
|
||||
assert "connection_configs" in inspector.get_table_names()
|
||||
# #endregion test_create_connection_bootstraps_missing_table
|
||||
# #endregion ConnectionsRoutesTests
|
||||
@@ -132,7 +132,7 @@ def test_get_datasets_invalid_pagination(mock_deps):
|
||||
# @RELATION BINDS_TO -> [DatasetsApiTests:Module]
|
||||
# @BRIEF Validate map-columns request creates an async mapping task and returns its identifier.
|
||||
# @TEST: POST /api/datasets/map-columns creates mapping task
|
||||
# @PRE: Valid env_id, dataset_ids, source_type
|
||||
# @PRE: Valid env_id, dataset_ids, source_type (sqllab)
|
||||
# @POST: Returns task_id
|
||||
def test_map_columns_success(mock_deps):
|
||||
# Mock environment
|
||||
@@ -145,7 +145,7 @@ def test_map_columns_success(mock_deps):
|
||||
mock_deps["task"].create_task = AsyncMock(return_value=mock_task)
|
||||
response = client.post(
|
||||
"/api/datasets/map-columns",
|
||||
json={"env_id": "prod", "dataset_ids": [1, 2, 3], "source_type": "postgresql"},
|
||||
json={"env_id": "prod", "dataset_ids": [1, 2, 3], "source_type": "sqllab", "database_id": 1},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
@@ -157,7 +157,7 @@ def test_map_columns_success(mock_deps):
|
||||
# @RELATION BINDS_TO -> [DatasetsApiTests:Module]
|
||||
# @BRIEF Validate map-columns rejects unsupported source types with a 400 contract response.
|
||||
# @TEST: POST /api/datasets/map-columns returns 400 for invalid source_type
|
||||
# @PRE: source_type is not 'postgresql' or 'xlsx'
|
||||
# @PRE: source_type is not 'sqllab' or 'xlsx'
|
||||
# @POST: Returns 400 error
|
||||
def test_map_columns_invalid_source_type(mock_deps):
|
||||
response = client.post(
|
||||
@@ -165,7 +165,7 @@ def test_map_columns_invalid_source_type(mock_deps):
|
||||
json={"env_id": "prod", "dataset_ids": [1], "source_type": "invalid"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
assert "Source type must be 'postgresql' or 'xlsx'" in response.json()["detail"]
|
||||
assert "Source type must be 'sqllab' or 'xlsx'" in response.json()["detail"]
|
||||
# #endregion test_map_columns_invalid_source_type
|
||||
# #region test_generate_docs_success [TYPE Function]
|
||||
# @RELATION BINDS_TO -> [DatasetsApiTests:Module]
|
||||
@@ -202,11 +202,24 @@ def test_map_columns_empty_ids(mock_deps):
|
||||
"""@PRE: dataset_ids must be non-empty."""
|
||||
response = client.post(
|
||||
"/api/datasets/map-columns",
|
||||
json={"env_id": "prod", "dataset_ids": [], "source_type": "postgresql"},
|
||||
json={"env_id": "prod", "dataset_ids": [], "source_type": "sqllab", "database_id": 1},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
assert "At least one dataset ID must be provided" in response.json()["detail"]
|
||||
# #endregion test_map_columns_empty_ids
|
||||
# #region test_map_columns_missing_database_id [TYPE Function]
|
||||
# @RELATION BINDS_TO -> [DatasetsApiTests:Module]
|
||||
# @BRIEF Validate map-columns rejects sqllab source without database_id.
|
||||
# @TEST: POST /api/datasets/map-columns returns 400 for sqllab without database_id
|
||||
# @POST: Returns 400 error
|
||||
def test_map_columns_missing_database_id(mock_deps):
|
||||
response = client.post(
|
||||
"/api/datasets/map-columns",
|
||||
json={"env_id": "prod", "dataset_ids": [1], "source_type": "sqllab"},
|
||||
)
|
||||
assert response.status_code == 400
|
||||
assert "database_id is required" in response.json()["detail"]
|
||||
# #endregion test_map_columns_missing_database_id
|
||||
# #region test_generate_docs_empty_ids [TYPE Function]
|
||||
# @RELATION BINDS_TO -> [DatasetsApiTests:Module]
|
||||
# @BRIEF Validate generate-docs rejects empty dataset identifier lists.
|
||||
|
||||
@@ -1,141 +0,0 @@
|
||||
# #region ConnectionsRouter [C:3] [TYPE Module] [SEMANTICS fastapi, connection, api, search]
|
||||
# @BRIEF Defines the FastAPI router for managing external database connections.
|
||||
# @LAYER: API
|
||||
# @RELATION DEPENDS_ON -> [get_db]
|
||||
# @RELATION DEPENDS_ON -> [ConnectionConfig]
|
||||
|
||||
from datetime import datetime
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from pydantic import BaseModel
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from ...core.database import ensure_connection_configs_table, get_db
|
||||
from ...core.logger import belief_scope, logger
|
||||
from ...models.connection import ConnectionConfig
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
# #region _ensure_connections_schema [C:3] [TYPE Function]
|
||||
# @BRIEF Ensures the connection_configs table exists before CRUD access.
|
||||
# @PRE: db is an active SQLAlchemy session.
|
||||
# @POST: The current bind can safely query ConnectionConfig.
|
||||
# @RELATION CALLS -> ensure_connection_configs_table
|
||||
def _ensure_connections_schema(db: Session):
|
||||
with belief_scope("ConnectionsRouter.ensure_schema"):
|
||||
ensure_connection_configs_table(db.get_bind())
|
||||
|
||||
|
||||
# #endregion _ensure_connections_schema
|
||||
|
||||
|
||||
# #region ConnectionSchema [C:3] [TYPE Class]
|
||||
# @BRIEF Pydantic model for connection response.
|
||||
# @RELATION BINDS_TO -> ConnectionConfig
|
||||
class ConnectionSchema(BaseModel):
|
||||
id: str
|
||||
name: str
|
||||
type: str
|
||||
host: str | None = None
|
||||
port: int | None = None
|
||||
database: str | None = None
|
||||
username: str | None = None
|
||||
created_at: datetime
|
||||
|
||||
class Config:
|
||||
orm_mode = True
|
||||
|
||||
|
||||
# #endregion ConnectionSchema
|
||||
|
||||
|
||||
# #region ConnectionCreate [C:3] [TYPE Class]
|
||||
# @BRIEF Pydantic model for creating a connection.
|
||||
# @RELATION BINDS_TO -> ConnectionConfig
|
||||
class ConnectionCreate(BaseModel):
|
||||
name: str
|
||||
type: str
|
||||
host: str | None = None
|
||||
port: int | None = None
|
||||
database: str | None = None
|
||||
username: str | None = None
|
||||
password: str | None = None
|
||||
|
||||
|
||||
# #endregion ConnectionCreate
|
||||
|
||||
|
||||
# #region list_connections [C:3] [TYPE Function]
|
||||
# @BRIEF Lists all saved connections.
|
||||
# @PRE: Database session is active.
|
||||
# @POST: Returns list of connection configs.
|
||||
# @RELATION CALLS -> _ensure_connections_schema
|
||||
# @RELATION DEPENDS_ON -> ConnectionConfig
|
||||
@router.get("", response_model=list[ConnectionSchema])
|
||||
async def list_connections(db: Session = Depends(get_db)):
|
||||
with belief_scope("ConnectionsRouter.list_connections"):
|
||||
_ensure_connections_schema(db)
|
||||
connections = db.query(ConnectionConfig).all()
|
||||
return connections
|
||||
|
||||
|
||||
# #endregion list_connections
|
||||
|
||||
|
||||
# #region create_connection [C:3] [TYPE Function]
|
||||
# @BRIEF Creates a new connection configuration.
|
||||
# @PRE: Connection name is unique.
|
||||
# @POST: Connection is saved to DB.
|
||||
# @RELATION CALLS -> _ensure_connections_schema
|
||||
# @RELATION DEPENDS_ON -> ConnectionConfig
|
||||
@router.post("", response_model=ConnectionSchema, status_code=status.HTTP_201_CREATED)
|
||||
async def create_connection(
|
||||
connection: ConnectionCreate, db: Session = Depends(get_db)
|
||||
):
|
||||
with belief_scope("ConnectionsRouter.create_connection", f"name={connection.name}"):
|
||||
_ensure_connections_schema(db)
|
||||
db_connection = ConnectionConfig(**connection.dict())
|
||||
db.add(db_connection)
|
||||
db.commit()
|
||||
db.refresh(db_connection)
|
||||
logger.info(
|
||||
f"[ConnectionsRouter.create_connection][Success] Created connection {db_connection.id}"
|
||||
)
|
||||
return db_connection
|
||||
|
||||
|
||||
# #endregion create_connection
|
||||
|
||||
|
||||
# #region delete_connection [C:3] [TYPE Function]
|
||||
# @BRIEF Deletes a connection configuration.
|
||||
# @PRE: Connection ID exists.
|
||||
# @POST: Connection is removed from DB.
|
||||
# @RELATION CALLS -> _ensure_connections_schema
|
||||
# @RELATION DEPENDS_ON -> ConnectionConfig
|
||||
@router.delete("/{connection_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||
async def delete_connection(connection_id: str, db: Session = Depends(get_db)):
|
||||
with belief_scope("ConnectionsRouter.delete_connection", f"id={connection_id}"):
|
||||
_ensure_connections_schema(db)
|
||||
db_connection = (
|
||||
db.query(ConnectionConfig)
|
||||
.filter(ConnectionConfig.id == connection_id)
|
||||
.first()
|
||||
)
|
||||
if not db_connection:
|
||||
logger.error(
|
||||
f"[ConnectionsRouter.delete_connection][State] Connection {connection_id} not found"
|
||||
)
|
||||
raise HTTPException(status_code=404, detail="Connection not found")
|
||||
db.delete(db_connection)
|
||||
db.commit()
|
||||
logger.info(
|
||||
f"[ConnectionsRouter.delete_connection][Success] Deleted connection {connection_id}"
|
||||
)
|
||||
return
|
||||
|
||||
|
||||
# #endregion delete_connection
|
||||
|
||||
# #endregion ConnectionsRouter
|
||||
@@ -316,8 +316,9 @@ async def get_datasets(
|
||||
class MapColumnsRequest(BaseModel):
|
||||
env_id: str = Field(..., description="Environment ID")
|
||||
dataset_ids: list[int] = Field(..., description="List of dataset IDs to map")
|
||||
source_type: str = Field(..., description="Source type: 'postgresql' or 'xlsx'")
|
||||
connection_id: str | None = Field(None, description="Connection ID for PostgreSQL source")
|
||||
source_type: str = Field(..., description="Source type: 'sqllab' or 'xlsx'")
|
||||
database_id: int | None = Field(None, description="Superset database ID for SQL Lab source")
|
||||
sql_query: str | None = Field(None, description="Custom SQL query for SQL Lab (optional, defaults to information_schema.columns query)")
|
||||
file_data: str | None = Field(None, description="File path or data for XLSX source")
|
||||
# #endregion MapColumnsRequest
|
||||
|
||||
@@ -344,9 +345,13 @@ async def map_columns(
|
||||
raise HTTPException(status_code=400, detail="At least one dataset ID must be provided")
|
||||
|
||||
# Validate source type
|
||||
if request.source_type not in ['postgresql', 'xlsx']:
|
||||
if request.source_type not in ['sqllab', 'xlsx']:
|
||||
logger.error(f"[map_columns][Coherence:Failed] Invalid source type: {request.source_type}")
|
||||
raise HTTPException(status_code=400, detail="Source type must be 'postgresql' or 'xlsx'")
|
||||
raise HTTPException(status_code=400, detail="Source type must be 'sqllab' or 'xlsx'")
|
||||
|
||||
# Validate database_id for sqllab source
|
||||
if request.source_type == 'sqllab' and not request.database_id:
|
||||
raise HTTPException(status_code=400, detail="database_id is required for 'sqllab' source type")
|
||||
|
||||
# Validate environment exists
|
||||
environments = config_manager.get_environments()
|
||||
@@ -361,8 +366,9 @@ async def map_columns(
|
||||
task_params = {
|
||||
'env': request.env_id,
|
||||
'dataset_id': request.dataset_ids[0] if request.dataset_ids else None,
|
||||
'source': request.source_type,
|
||||
'connection_id': request.connection_id,
|
||||
'source': 'sqllab' if request.source_type == 'sqllab' else 'excel',
|
||||
'database_id': request.database_id,
|
||||
'sql_query': request.sql_query,
|
||||
'file_data': request.file_data
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user