-
Notifications
You must be signed in to change notification settings - Fork 46.1k
feat(backend): add SQLAlchemy infrastructure for database operations #11419
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
Open
Swiftyos
wants to merge
8
commits into
dev
Choose a base branch
from
swiftyos/sqlalchemy-plumbing
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,284
−1
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
5c8dac4
added sqlalchemy plumbing
Swiftyos 0085e5e
integrate into database manager and rest api.
Swiftyos 900b0e7
add tests
Swiftyos 8ae5cbe
added tests and toggle for sqlalchemy integration
Swiftyos 5388a32
update poetry lock
Swiftyos 0c0488e
Added docs and more specific error handling
Swiftyos b114354
update asyncpg to version ^0.30.0
Swiftyos 39839a5
Merge branch 'dev' into swiftyos/sqlalchemy-plumbing
Swiftyos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| """ | ||
| SQLAlchemy infrastructure for AutoGPT Platform. | ||
|
|
||
| This module provides: | ||
| 1. Async engine creation with connection pooling | ||
| 2. Session factory for dependency injection | ||
| 3. Database lifecycle management | ||
| """ | ||
|
|
||
| import logging | ||
| import re | ||
| from contextlib import asynccontextmanager | ||
| from typing import AsyncGenerator | ||
|
|
||
| from sqlalchemy.ext.asyncio import ( | ||
| AsyncEngine, | ||
| AsyncSession, | ||
| async_sessionmaker, | ||
| create_async_engine, | ||
| ) | ||
|
|
||
| from backend.util.settings import Config | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # ============================================================================ | ||
| # CONFIGURATION | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| def get_database_url() -> str: | ||
| """ | ||
| Extract database URL from environment and convert to async format. | ||
|
|
||
| Prisma URL: postgresql://user:pass@host:port/db?schema=platform&connect_timeout=60 | ||
| Async URL: postgresql+asyncpg://user:pass@host:port/db | ||
|
|
||
| Returns the async-compatible URL without query parameters (handled via connect_args). | ||
| """ | ||
| prisma_url = Config().database_url | ||
|
|
||
| # Replace postgresql:// with postgresql+asyncpg:// | ||
| async_url = prisma_url.replace("postgresql://", "postgresql+asyncpg://") | ||
|
|
||
| # Remove ALL query parameters (schema, connect_timeout, etc.) | ||
| # We'll handle these through connect_args instead | ||
| async_url = re.sub(r"\?.*$", "", async_url) | ||
|
|
||
| return async_url | ||
|
|
||
|
|
||
| def get_database_schema() -> str: | ||
| """ | ||
| Extract schema name from DATABASE_URL query parameter. | ||
|
|
||
| Returns 'platform' by default (matches Prisma configuration). | ||
| """ | ||
| prisma_url = Config().database_url | ||
| match = re.search(r"schema=(\w+)", prisma_url) | ||
| return match.group(1) if match else "platform" | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # ENGINE CREATION | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| def create_engine() -> AsyncEngine: | ||
| """ | ||
| Create async SQLAlchemy engine with connection pooling. | ||
|
|
||
| This should be called ONCE per process at startup. | ||
| The engine is long-lived and thread-safe. | ||
|
|
||
| Connection Pool Configuration: | ||
| - pool_size: Number of persistent connections (default: 10) | ||
| - max_overflow: Additional connections when pool exhausted (default: 5) | ||
| - pool_timeout: Seconds to wait for connection (default: 30) | ||
| - pool_pre_ping: Test connections before using (prevents stale connections) | ||
|
|
||
| Total max connections = pool_size + max_overflow = 15 | ||
| """ | ||
| url = get_database_url() | ||
| config = Config() | ||
|
|
||
| engine = create_async_engine( | ||
| url, | ||
| # Connection pool configuration | ||
| pool_size=config.sqlalchemy_pool_size, # Persistent connections | ||
| max_overflow=config.sqlalchemy_max_overflow, # Burst capacity | ||
| pool_timeout=config.sqlalchemy_pool_timeout, # Wait time for connection | ||
| pool_pre_ping=True, # Validate connections before use | ||
| # Async configuration | ||
| echo=config.sqlalchemy_echo, # Log SQL statements (dev/debug only) | ||
| future=True, # Use SQLAlchemy 2.0 style | ||
| # Connection arguments (passed to asyncpg) | ||
| connect_args={ | ||
| "server_settings": { | ||
| "search_path": get_database_schema(), # Use 'platform' schema | ||
| }, | ||
| "timeout": config.sqlalchemy_connect_timeout, # Connection timeout | ||
| }, | ||
| ) | ||
|
|
||
| logger.info( | ||
| f"SQLAlchemy engine created: pool_size={config.sqlalchemy_pool_size}, " | ||
| f"max_overflow={config.sqlalchemy_max_overflow}, " | ||
| f"schema={get_database_schema()}" | ||
| ) | ||
|
|
||
| return engine | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # SESSION FACTORY | ||
| # ============================================================================ | ||
|
|
||
|
|
||
| def create_session_factory(engine: AsyncEngine) -> async_sessionmaker[AsyncSession]: | ||
| """ | ||
| Create session factory for creating AsyncSession instances. | ||
|
|
||
| The factory is configured once, then used to create sessions on-demand. | ||
| Each session represents a single database transaction. | ||
|
|
||
| Args: | ||
| engine: The async engine (with connection pool) | ||
|
|
||
| Returns: | ||
| Session factory that creates properly configured AsyncSession instances | ||
| """ | ||
| return async_sessionmaker( | ||
| bind=engine, | ||
| class_=AsyncSession, | ||
| expire_on_commit=False, # Don't expire objects after commit | ||
| autoflush=False, # Manual control over when to flush | ||
| autocommit=False, # Explicit transaction control | ||
| ) | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # DEPENDENCY INJECTION FOR FASTAPI | ||
| # ============================================================================ | ||
|
|
||
| # Global references (set during app startup) | ||
| _engine: AsyncEngine | None = None | ||
| _session_factory: async_sessionmaker[AsyncSession] | None = None | ||
|
|
||
|
|
||
| def initialize(engine: AsyncEngine) -> None: | ||
| """ | ||
| Initialize global engine and session factory. | ||
|
|
||
| Called during FastAPI lifespan startup. | ||
|
|
||
| Args: | ||
| engine: The async engine to use for this process | ||
| """ | ||
| global _engine, _session_factory | ||
| _engine = engine | ||
| _session_factory = create_session_factory(engine) | ||
| logger.info("SQLAlchemy session factory initialized") | ||
|
|
||
|
|
||
| @asynccontextmanager | ||
| async def get_session() -> AsyncGenerator[AsyncSession, None]: | ||
| """ | ||
| FastAPI dependency that provides database session. | ||
|
|
||
| Usage in routes: | ||
| @router.get("/users/{user_id}") | ||
| async def get_user( | ||
| user_id: int, | ||
| session: AsyncSession = Depends(get_session) | ||
| ): | ||
| result = await session.execute(select(User).where(User.id == user_id)) | ||
| return result.scalar_one_or_none() | ||
|
|
||
| Usage in DatabaseManager RPC methods: | ||
| @expose | ||
| async def get_user(user_id: int): | ||
| async with get_session() as session: | ||
| result = await session.execute(select(User).where(User.id == user_id)) | ||
| return result.scalar_one_or_none() | ||
|
|
||
| Lifecycle: | ||
| 1. Request arrives | ||
| 2. FastAPI calls this function (or used as context manager) | ||
| 3. Session is created (borrows connection from pool) | ||
| 4. Session is injected into route handler | ||
| 5. Route executes (may commit/rollback) | ||
| 6. Route returns | ||
| 7. Session is closed (returns connection to pool) | ||
|
|
||
| Error handling: | ||
| - If exception occurs, session is rolled back | ||
| - Connection is always returned to pool (even on error) | ||
| """ | ||
| if _session_factory is None: | ||
| raise RuntimeError( | ||
| "SQLAlchemy not initialized. Call initialize() in lifespan context." | ||
| ) | ||
|
|
||
| # Create session (borrows connection from pool) | ||
| async with _session_factory() as session: | ||
| try: | ||
| yield session # Inject into route handler or context manager | ||
| # If we get here, route succeeded - commit any pending changes | ||
| await session.commit() | ||
| except Exception: | ||
| # Error occurred - rollback transaction | ||
| await session.rollback() | ||
| raise | ||
| finally: | ||
| # Always close session (returns connection to pool) | ||
| await session.close() | ||
|
|
||
|
|
||
| async def dispose() -> None: | ||
| """ | ||
| Dispose of engine and close all connections. | ||
|
|
||
| Called during FastAPI lifespan shutdown. | ||
| Closes all connections in the pool gracefully. | ||
| """ | ||
| global _engine, _session_factory | ||
|
|
||
| if _engine is not None: | ||
| logger.info("Disposing SQLAlchemy engine...") | ||
| await _engine.dispose() | ||
| _engine = None | ||
| _session_factory = None | ||
| logger.info("SQLAlchemy engine disposed") | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Bug: Empty
DATABASE_URLwithenable_sqlalchemy=truecauses unhandledArgumentErrorduring startup.Severity: HIGH | Confidence: High
🔍 Detailed Analysis
When
enable_sqlalchemy=trueis set and theDATABASE_URLenvironment variable is not provided, theConfig().database_urldefaults to an empty string. Theget_database_url()function then attempts to create an engine with this empty URL, causing SQLAlchemy to raise anArgumentError. ThisArgumentErroris not caught by the existing exception handlers indatabase.pyorrest_api.py, leading to an unhandled exception and application crash during startup.💡 Suggested Fix
Validate that
database_urlis not empty before callingcreate_async_engine(), or addArgumentErrorto the list of caught exceptions, or set a sensible fallback fordatabase_url.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2841823