From 98a8384782f9b0b4483dbb858adbf15ab4e3be6c Mon Sep 17 00:00:00 2001 From: Matembu Emmanuel Dominic Date: Fri, 5 Jul 2024 19:03:59 +0300 Subject: [PATCH] fix: update env.py to use SQLALCHEMY_DATABASE_URI from Flask config and prevent empty migrations - Retrieve SQLALCHEMY_DATABASE_URI from Flask app config based on environment - Add process_revision_directives function to prevent generating empty migration scripts - Log a message when no schema changes are detected - Ensure database URI is correctly set in Alembic config - Update user_role model id to uuid to prevent migration warning - Fix CI to ensure proper database setup for tests --- .github/workflows/test.yml | 11 +++- alembic.ini | 6 +- app/helpers/age_utility.py | 14 ++--- app/models/model_mixin.py | 3 - app/models/user_role.py | 4 +- app/tests/conftest.py | 2 - migrations/env.py | 93 +++++++++++++--------------- migrations/versions/605e99996497_.py | 2 +- migrations/versions/f8d3ac1848b9_.py | 45 ++++++++++++++ server.py | 10 +++ 10 files changed, 121 insertions(+), 69 deletions(-) create mode 100644 migrations/versions/f8d3ac1848b9_.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 47ce4133..8c87349e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,7 +38,7 @@ jobs: strategy: matrix: - python-version: ["3.8", "3.9", "3.10"] + python-version: ["3.9", "3.10", "3.11"] steps: - uses: actions/checkout@v2 @@ -52,6 +52,15 @@ jobs: pip install pytest if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + - name: Wait for PostgreSQL to be ready + run: | + echo "Waiting for PostgreSQL to be ready..." + until pg_isready -h localhost -p 5432; do sleep 1; done + + - name: Install uuid-ossp extension + run: | + PGPASSWORD=${{ secrets.TEST_DATABASE_PASSWORD }} psql -h localhost -U postgres -d ${{ secrets.TEST_DATABASE_NAME }} -c "CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\";" + - name: Test with pytest env: TEST_DATABASE_URI: ${{ secrets.TEST_DATABASE_URI }} diff --git a/alembic.ini b/alembic.ini index cbb5bac8..08a55518 100644 --- a/alembic.ini +++ b/alembic.ini @@ -76,7 +76,7 @@ version_path_separator = os # Use os.pathsep. Default configuration used for ne # ruff.type = exec # ruff.executable = %(here)s/.venv/bin/ruff # ruff.options = --fix REVISION_SCRIPT_FILENAME -url = 'postgresql+psycopg2://postgres:postgres@localhost:5432/cranecloud' +# url = '%(DATABASE_URI)s' # Logging configuration [loggers] @@ -95,12 +95,12 @@ qualname = [logger_sqlalchemy] level = WARN -handlers = +handlers = console qualname = sqlalchemy.engine [logger_alembic] level = INFO -handlers = +handlers = console qualname = alembic [handler_console] diff --git a/app/helpers/age_utility.py b/app/helpers/age_utility.py index 01e75883..0d1008a5 100644 --- a/app/helpers/age_utility.py +++ b/app/helpers/age_utility.py @@ -1,20 +1,20 @@ from datetime import datetime -def get_item_age(time=None): +def get_item_age(time=False): """ - Get a datetime object or an int() Epoch timestamp and return a + Get a datetime object or a int() Epoch timestamp and return a pretty string like 'an hour ago', 'Yesterday', '3 months ago', - 'just now', etc. + 'just now', etc """ + diff = 0 now = datetime.now() - - if time is None: - return "Invalid time provided" + if not time: + diff = now - now diff = now - time - + second_diff = diff.seconds day_diff = diff.days diff --git a/app/models/model_mixin.py b/app/models/model_mixin.py index 282769b2..c28fe0ae 100644 --- a/app/models/model_mixin.py +++ b/app/models/model_mixin.py @@ -1,7 +1,6 @@ from sqlalchemy import inspect, func, column from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm import Query -from sqlalchemy import or_ from ..models import db from sqlalchemy import or_ import time @@ -35,8 +34,6 @@ def enable_deleted(self): return self - - class ModelMixin(db.Model): __abstract__ = True diff --git a/app/models/user_role.py b/app/models/user_role.py index bb1f97eb..c1b05d1b 100644 --- a/app/models/user_role.py +++ b/app/models/user_role.py @@ -1,6 +1,6 @@ +import uuid from app.models import db from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import relationship, backref from app.models.user import User from app.models.role import Role from app.models.model_mixin import ModelMixin @@ -9,6 +9,6 @@ class UserRole(ModelMixin): _tablename_ = "user_roles" - id = db.Column(db.Integer, primary_key=True) + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) user_id = db.Column('user_id', UUID(as_uuid=True), db.ForeignKey(User.id)) role_id = db.Column("role_id", UUID(as_uuid=True), db.ForeignKey(Role.id)) diff --git a/app/tests/conftest.py b/app/tests/conftest.py index 2e1fd9ad..84d63f9d 100644 --- a/app/tests/conftest.py +++ b/app/tests/conftest.py @@ -3,9 +3,7 @@ from app.helpers.admin import create_default_roles from app.schemas.user import UserSchema from app.tests.user import UserBaseTestCase -from sqlalchemy import text import pytest -import logging from app.models.user import User from server import create_app, db diff --git a/migrations/env.py b/migrations/env.py index e65e381c..7ca012a5 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -1,71 +1,64 @@ import os from logging.config import fileConfig -import logging -from sqlalchemy import engine_from_config, pool, text +from sqlalchemy import engine_from_config, pool from alembic import context config = context.config -if config.config_file_name is not None: - fileConfig(config.config_file_name) - -logger = logging.getLogger('alembic.env') +fileConfig(config.config_file_name) from server import create_app -from app.models import db config_name = os.getenv('FLASK_CONFIG') or 'development' app = create_app(config_name) -with app.app_context(): - config.set_main_option( - 'sqlalchemy.url', app.config.get('SQLALCHEMY_DATABASE_URI').replace('%', '%%')) - target_metadata = app.extensions['migrate'].db.metadata +database_uri = app.config['SQLALCHEMY_DATABASE_URI'] +if not database_uri: + raise ValueError("SQLALCHEMY_DATABASE_URI is not set") - def run_migrations_offline() -> None: - url = config.get_main_option("sqlalchemy.url") - context.configure( - url=url, - target_metadata=target_metadata, - literal_binds=True, - dialect_opts={"paramstyle": "named"}, - ) +config.set_main_option('sqlalchemy.url', database_uri.replace('%', '%%')) +target_metadata = app.extensions['migrate'].db.metadata - with context.begin_transaction(): - context.run_migrations() +def process_revision_directives(context, revision, directives): + if getattr(config.cmd_opts, 'autogenerate', False): + script = directives[0] + if script.upgrade_ops.is_empty(): + directives[:] = [] + app.logger.info('No changes in schema detected.') - def run_migrations_online() -> None: - def process_revision_directives(context, revision, directives): - if getattr(config.cmd_opts, 'autogenerate', False): - script = directives[0] - if script.upgrade_ops.is_empty(): - directives[:] = [] - logger.info('No changes in schema detected.') +def run_migrations_offline(): + url = config.get_main_option("sqlalchemy.url") + context.configure( + url=url, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + process_revision_directives=process_revision_directives, + ) - connectable = engine_from_config( - config.get_section(config.config_ini_section), - prefix='sqlalchemy.', - poolclass=pool.NullPool, - ) + with context.begin_transaction(): + context.run_migrations() - with connectable.connect() as connection: - context.configure( - connection=connection, - target_metadata=target_metadata, - process_revision_directives=process_revision_directives, - **app.extensions['migrate'].configure_args - ) +def run_migrations_online(): + connectable = engine_from_config( + config.get_section(config.config_ini_section), + prefix="sqlalchemy.", + poolclass=pool.NullPool, + ) - logger.info('Creating extension uuid-ossp...') - connection.execute(text('CREATE EXTENSION IF NOT EXISTS "uuid-ossp"')) - logger.info('Extension uuid-ossp created.') + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + process_revision_directives=process_revision_directives, + ) - with context.begin_transaction(): - context.run_migrations() - logger.info('Migrations have been run.') + with context.begin_transaction(): + context.run_migrations() + app.logger.info('Migrations have been run.') - if context.is_offline_mode(): - run_migrations_offline() - else: - run_migrations_online() +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/migrations/versions/605e99996497_.py b/migrations/versions/605e99996497_.py index bf8e6af9..9960871e 100644 --- a/migrations/versions/605e99996497_.py +++ b/migrations/versions/605e99996497_.py @@ -78,7 +78,7 @@ def upgrade(): sa.PrimaryKeyConstraint('id') ) op.create_table('user_role', - sa.Column('id', sa.Integer(), nullable=False), + sa.Column('id', postgresql.UUID(as_uuid=True), primary_key=True, server_default=sa.text('uuid_generate_v4()')), sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), sa.Column('role_id', postgresql.UUID(as_uuid=True), nullable=True), sa.ForeignKeyConstraint(['role_id'], ['role.id'], ), diff --git a/migrations/versions/f8d3ac1848b9_.py b/migrations/versions/f8d3ac1848b9_.py new file mode 100644 index 00000000..4d4a8a51 --- /dev/null +++ b/migrations/versions/f8d3ac1848b9_.py @@ -0,0 +1,45 @@ +"""empty message + +Revision ID: f8d3ac1848b9 +Revises: 1888b16f6b99 +Create Date: 2024-07-04 22:52:34.174913 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'f8d3ac1848b9' +down_revision = '1888b16f6b99' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + # Add a new column with UUID type + op.add_column('user_role', sa.Column('new_id', postgresql.UUID(as_uuid=True), nullable=False, server_default=sa.text('uuid_generate_v4()'))) + # Copy data from old id column to new_id column (UUIDs will be generated) + op.execute('UPDATE user_role SET new_id = uuid_generate_v4()') + # Drop the old id column + op.drop_column('user_role', 'id') + # Rename the new_id column to id + op.alter_column('user_role', 'new_id', new_column_name='id') + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + + # Add the old id column back with INTEGER type + op.add_column('user_role', sa.Column('old_id', sa.Integer, nullable=False, server_default=sa.text('nextval(\'user_role_id_seq\'::regclass)'))) + # Note: Data copying back from UUID to INTEGER is not straightforward + # Skipping data copying for downgrade + # Drop the new id (UUID) column + op.drop_column('user_role', 'id') + # Rename the old_id column to id + op.alter_column('user_role', 'old_id', new_column_name='id') + + # ### end Alembic commands ### diff --git a/server.py b/server.py index 81687e1f..a429b9cc 100644 --- a/server.py +++ b/server.py @@ -7,6 +7,8 @@ from flask_jwt_extended import JWTManager from flasgger import Swagger from flask_migrate import Migrate +from sqlalchemy import event +from sqlalchemy.engine import Engine from app.routes import api from manage import admin_user, create_registries, create_roles @@ -111,6 +113,14 @@ def user_identity_lookup(user): # Celery celery = update_celery(app) +@event.listens_for(Engine, "connect") +def create_postgres_extension(dbapi_connection, connection_record): + cursor = dbapi_connection.cursor() + try: + cursor.execute("CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\";") + except Exception as e: + app.logger.error(f"Failed to create PostgreSQL extension: {e}") + cursor.close() if __name__ == '__main__': app.run()