From 025900c84949f32d7d5cf88e6366a827a0e48b6d Mon Sep 17 00:00:00 2001 From: Automated Action Date: Tue, 24 Jun 2025 19:46:05 +0000 Subject: [PATCH] Fix database migration conflicts and improve migration safety Changes: - Enhanced migration 002 with column existence checks to prevent duplicate column errors - Added comprehensive migration 003 that syncs database with current model state - Modified application startup to avoid conflicts between create_all() and Alembic - Added proper table/column existence checking in migrations - Improved migration safety for production environments - Removed automatic table creation when database already exists (relies on migrations) This resolves the 'duplicate column name' error by ensuring migrations check for existing columns before attempting to add them. --- alembic/versions/002_add_profile_fields.py | 63 +++++++++---- alembic/versions/003_sync_current_state.py | 102 +++++++++++++++++++++ main.py | 16 +++- 3 files changed, 159 insertions(+), 22 deletions(-) create mode 100644 alembic/versions/003_sync_current_state.py diff --git a/alembic/versions/002_add_profile_fields.py b/alembic/versions/002_add_profile_fields.py index f28c4a5..5daeaf2 100644 --- a/alembic/versions/002_add_profile_fields.py +++ b/alembic/versions/002_add_profile_fields.py @@ -9,6 +9,7 @@ from typing import Sequence, Union from alembic import op import sqlalchemy as sa +from sqlalchemy.engine.reflection import Inspector # revision identifiers, used by Alembic. revision: str = '002' @@ -17,27 +18,51 @@ branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None +def column_exists(table_name, column_name): + """Check if a column exists in a table""" + bind = op.get_bind() + inspector = Inspector.from_engine(bind) + columns = [c['name'] for c in inspector.get_columns(table_name)] + return column_name in columns + + def upgrade() -> None: - # Add new profile fields to users table - op.add_column('users', sa.Column('first_name', sa.String(), nullable=True)) - op.add_column('users', sa.Column('last_name', sa.String(), nullable=True)) - op.add_column('users', sa.Column('phone', sa.String(), nullable=True)) - op.add_column('users', sa.Column('bio', sa.String(), nullable=True)) - op.add_column('users', sa.Column('preferred_language', sa.String(), nullable=True)) - op.add_column('users', sa.Column('timezone', sa.String(), nullable=True)) - op.add_column('users', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('(CURRENT_TIMESTAMP)'), nullable=True)) + # List of columns to add + columns_to_add = [ + ('first_name', sa.Column('first_name', sa.String(), nullable=True)), + ('last_name', sa.Column('last_name', sa.String(), nullable=True)), + ('phone', sa.Column('phone', sa.String(), nullable=True)), + ('bio', sa.Column('bio', sa.String(), nullable=True)), + ('preferred_language', sa.Column('preferred_language', sa.String(), nullable=True)), + ('timezone', sa.Column('timezone', sa.String(), nullable=True)), + ('updated_at', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('(CURRENT_TIMESTAMP)'), nullable=True)) + ] - # Set default values for existing users - op.execute("UPDATE users SET preferred_language = 'en' WHERE preferred_language IS NULL") - op.execute("UPDATE users SET timezone = 'UTC' WHERE timezone IS NULL") + # Add columns only if they don't exist + for column_name, column_def in columns_to_add: + if not column_exists('users', column_name): + op.add_column('users', column_def) + + # Set default values for existing users (only if columns exist) + if column_exists('users', 'preferred_language'): + op.execute("UPDATE users SET preferred_language = 'en' WHERE preferred_language IS NULL") + if column_exists('users', 'timezone'): + op.execute("UPDATE users SET timezone = 'UTC' WHERE timezone IS NULL") def downgrade() -> None: - # Remove the added columns - op.drop_column('users', 'updated_at') - op.drop_column('users', 'timezone') - op.drop_column('users', 'preferred_language') - op.drop_column('users', 'bio') - op.drop_column('users', 'phone') - op.drop_column('users', 'last_name') - op.drop_column('users', 'first_name') \ No newline at end of file + # List of columns to remove (in reverse order) + columns_to_remove = [ + 'updated_at', + 'timezone', + 'preferred_language', + 'bio', + 'phone', + 'last_name', + 'first_name' + ] + + # Remove columns only if they exist + for column_name in columns_to_remove: + if column_exists('users', column_name): + op.drop_column('users', column_name) \ No newline at end of file diff --git a/alembic/versions/003_sync_current_state.py b/alembic/versions/003_sync_current_state.py new file mode 100644 index 0000000..a9ebfb9 --- /dev/null +++ b/alembic/versions/003_sync_current_state.py @@ -0,0 +1,102 @@ +"""Sync database with current model state + +Revision ID: 003 +Revises: 002 +Create Date: 2024-01-01 00:00:02.000000 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.engine.reflection import Inspector + +# revision identifiers, used by Alembic. +revision: str = '003' +down_revision: Union[str, None] = '002' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def table_exists(table_name): + """Check if a table exists""" + bind = op.get_bind() + inspector = Inspector.from_engine(bind) + return table_name in inspector.get_table_names() + + +def column_exists(table_name, column_name): + """Check if a column exists in a table""" + if not table_exists(table_name): + return False + bind = op.get_bind() + inspector = Inspector.from_engine(bind) + columns = [c['name'] for c in inspector.get_columns(table_name)] + return column_name in columns + + +def upgrade() -> None: + """ + This migration ensures the database is in sync with the current model state. + It safely handles the case where tables/columns may already exist. + """ + + # Check if users table exists, if not create it with all fields + if not table_exists('users'): + op.create_table('users', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('email', sa.String(), nullable=False), + sa.Column('password_hash', sa.String(), nullable=False), + sa.Column('first_name', sa.String(), nullable=True), + sa.Column('last_name', sa.String(), nullable=True), + sa.Column('phone', sa.String(), nullable=True), + sa.Column('bio', sa.String(), nullable=True), + sa.Column('preferred_language', sa.String(), nullable=True), + sa.Column('timezone', sa.String(), nullable=True), + sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('(CURRENT_TIMESTAMP)'), nullable=True), + sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('(CURRENT_TIMESTAMP)'), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_users_email'), 'users', ['email'], unique=True) + op.create_index(op.f('ix_users_id'), 'users', ['id'], unique=False) + else: + # Table exists, check for missing profile columns and add them + profile_columns = [ + ('first_name', sa.Column('first_name', sa.String(), nullable=True)), + ('last_name', sa.Column('last_name', sa.String(), nullable=True)), + ('phone', sa.Column('phone', sa.String(), nullable=True)), + ('bio', sa.Column('bio', sa.String(), nullable=True)), + ('preferred_language', sa.Column('preferred_language', sa.String(), nullable=True)), + ('timezone', sa.Column('timezone', sa.String(), nullable=True)), + ('updated_at', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('(CURRENT_TIMESTAMP)'), nullable=True)) + ] + + for column_name, column_def in profile_columns: + if not column_exists('users', column_name): + op.add_column('users', column_def) + + # Set default values for profile fields + if column_exists('users', 'preferred_language'): + op.execute("UPDATE users SET preferred_language = 'en' WHERE preferred_language IS NULL") + if column_exists('users', 'timezone'): + op.execute("UPDATE users SET timezone = 'UTC' WHERE timezone IS NULL") + + +def downgrade() -> None: + """ + Downgrade removes the profile fields added in this migration + """ + if table_exists('users'): + profile_columns = [ + 'updated_at', + 'timezone', + 'preferred_language', + 'bio', + 'phone', + 'last_name', + 'first_name' + ] + + for column_name in profile_columns: + if column_exists('users', column_name): + op.drop_column('users', column_name) \ No newline at end of file diff --git a/main.py b/main.py index e1b3752..28b4685 100644 --- a/main.py +++ b/main.py @@ -16,14 +16,24 @@ logger = logging.getLogger(__name__) @asynccontextmanager async def lifespan(app: FastAPI): try: - # Create database tables + # Create database directory logger.info("Starting application initialization...") storage_dir = Path("/app/storage/db") storage_dir.mkdir(parents=True, exist_ok=True) logger.info(f"Created storage directory: {storage_dir}") - Base.metadata.create_all(bind=engine) - logger.info("Database tables created successfully") + # Import all models to ensure they're registered with Base + + # Only create tables if database doesn't exist (for development) + # In production, use Alembic migrations instead + db_path = storage_dir / "db.sqlite" + if not db_path.exists(): + logger.info("Database file doesn't exist, creating initial tables...") + Base.metadata.create_all(bind=engine) + else: + logger.info("Database file exists, skipping table creation (use Alembic for migrations)") + + logger.info("Database initialization completed") yield except Exception as e: logger.error(f"Error during application startup: {e}")