diff --git a/.github/workflows/test-assets.yml b/.github/workflows/test-assets.yml new file mode 100644 index 000000000..3b3a7c73f --- /dev/null +++ b/.github/workflows/test-assets.yml @@ -0,0 +1,174 @@ +name: Asset System Tests + +on: + push: + paths: + - 'app/**' + - 'alembic_db/**' + - 'tests-assets/**' + - '.github/workflows/test-assets.yml' + - 'requirements.txt' + pull_request: + branches: [master] + workflow_dispatch: + +permissions: + contents: read + +env: + PIP_DISABLE_PIP_VERSION_CHECK: '1' + PYTHONUNBUFFERED: '1' + +jobs: + sqlite: + name: SQLite (${{ matrix.sqlite_mode }}) • Python ${{ matrix.python }} + runs-on: ubuntu-latest + timeout-minutes: 40 + strategy: + fail-fast: false + matrix: + python: ['3.9', '3.12'] + sqlite_mode: ['memory', 'file'] + + steps: + - uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + + - name: Install dependencies + run: | + python -m pip install -U pip wheel + pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu + pip install -r requirements.txt + pip install pytest pytest-aiohttp pytest-asyncio + + - name: Set deterministic test base dir + id: basedir + shell: bash + run: | + BASE="$RUNNER_TEMP/comfyui-assets-tests-${{ matrix.python }}-${{ matrix.sqlite_mode }}-${{ github.run_id }}-${{ github.run_attempt }}" + echo "ASSETS_TEST_BASE_DIR=$BASE" >> "$GITHUB_ENV" + echo "ASSETS_TEST_LOGS=$BASE/logs" >> "$GITHUB_ENV" + mkdir -p "$BASE/logs" + echo "ASSETS_TEST_BASE_DIR=$BASE" + + - name: Set DB URL for SQLite + id: setdb + shell: bash + run: | + if [ "${{ matrix.sqlite_mode }}" = "memory" ]; then + echo "ASSETS_TEST_DB_URL=sqlite+aiosqlite:///:memory:" >> "$GITHUB_ENV" + else + DBFILE="$RUNNER_TEMP/assets-tests.sqlite" + mkdir -p "$(dirname "$DBFILE")" + echo "ASSETS_TEST_DB_URL=sqlite+aiosqlite:///$DBFILE" >> "$GITHUB_ENV" + fi + + - name: Run tests + run: python -m pytest tests-assets + + - name: Show ComfyUI logs + if: always() + shell: bash + run: | + echo "==== ASSETS_TEST_BASE_DIR: $ASSETS_TEST_BASE_DIR ====" + echo "==== ASSETS_TEST_LOGS: $ASSETS_TEST_LOGS ====" + ls -la "$ASSETS_TEST_LOGS" || true + for f in "$ASSETS_TEST_LOGS"/stdout.log "$ASSETS_TEST_LOGS"/stderr.log; do + if [ -f "$f" ]; then + echo "----- BEGIN $f -----" + sed -n '1,400p' "$f" + echo "----- END $f -----" + fi + done + + - name: Upload ComfyUI logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: asset-logs-sqlite-${{ matrix.sqlite_mode }}-py${{ matrix.python }} + path: ${{ env.ASSETS_TEST_LOGS }}/*.log + if-no-files-found: warn + + postgres: + name: PostgreSQL ${{ matrix.pgsql }} • Python ${{ matrix.python }} + runs-on: ubuntu-latest + timeout-minutes: 40 + strategy: + fail-fast: false + matrix: + python: ['3.9', '3.12'] + pgsql: ['14', '16'] + + services: + postgres: + image: postgres:${{ matrix.pgsql }} + env: + POSTGRES_DB: assets + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres -d assets" + --health-interval 10s + --health-timeout 5s + --health-retries 12 + + steps: + - uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + + - name: Install dependencies + run: | + python -m pip install -U pip wheel + pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu + pip install -r requirements.txt + pip install pytest pytest-aiohttp pytest-asyncio + pip install greenlet psycopg + + - name: Set deterministic test base dir + id: basedir + shell: bash + run: | + BASE="$RUNNER_TEMP/comfyui-assets-tests-${{ matrix.python }}-${{ matrix.sqlite_mode }}-${{ github.run_id }}-${{ github.run_attempt }}" + echo "ASSETS_TEST_BASE_DIR=$BASE" >> "$GITHUB_ENV" + echo "ASSETS_TEST_LOGS=$BASE/logs" >> "$GITHUB_ENV" + mkdir -p "$BASE/logs" + echo "ASSETS_TEST_BASE_DIR=$BASE" + + - name: Set DB URL for PostgreSQL + shell: bash + run: | + echo "ASSETS_TEST_DB_URL=postgresql+psycopg://postgres:postgres@localhost:5432/assets" >> "$GITHUB_ENV" + + - name: Run tests + run: python -m pytest tests-assets + + - name: Show ComfyUI logs + if: always() + shell: bash + run: | + echo "==== ASSETS_TEST_BASE_DIR: $ASSETS_TEST_BASE_DIR ====" + echo "==== ASSETS_TEST_LOGS: $ASSETS_TEST_LOGS ====" + ls -la "$ASSETS_TEST_LOGS" || true + for f in "$ASSETS_TEST_LOGS"/stdout.log "$ASSETS_TEST_LOGS"/stderr.log; do + if [ -f "$f" ]; then + echo "----- BEGIN $f -----" + sed -n '1,400p' "$f" + echo "----- END $f -----" + fi + done + + - name: Upload ComfyUI logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: asset-logs-pgsql-${{ matrix.pgsql }}-py${{ matrix.python }} + path: ${{ env.ASSETS_TEST_LOGS }}/*.log + if-no-files-found: warn diff --git a/alembic_db/versions/0001_assets.py b/alembic_db/versions/0001_assets.py index 9481100b0..f3b3ee0bf 100644 --- a/alembic_db/versions/0001_assets.py +++ b/alembic_db/versions/0001_assets.py @@ -7,6 +7,7 @@ Create Date: 2025-08-20 00:00:00 from alembic import op import sqlalchemy as sa +from sqlalchemy.dialects import postgresql revision = "0001_assets" down_revision = None @@ -90,7 +91,7 @@ def upgrade() -> None: sa.Column("val_str", sa.String(length=2048), nullable=True), sa.Column("val_num", sa.Numeric(38, 10), nullable=True), sa.Column("val_bool", sa.Boolean(), nullable=True), - sa.Column("val_json", sa.JSON(), nullable=True), + sa.Column("val_json", sa.JSON().with_variant(postgresql.JSONB(), 'postgresql'), nullable=True), sa.PrimaryKeyConstraint("asset_info_id", "key", "ordinal", name="pk_asset_info_meta"), ) op.create_index("ix_asset_info_meta_key", "asset_info_meta", ["key"]) diff --git a/app/database/db.py b/app/database/db.py index eaf6648db..8280272b0 100644 --- a/app/database/db.py +++ b/app/database/db.py @@ -69,23 +69,6 @@ def _normalize_sqlite_memory_url(db_url: str) -> tuple[str, bool]: return str(u), False -def _to_sync_driver_url(async_url: str) -> str: - """Convert an async SQLAlchemy URL to a sync URL for Alembic.""" - u = make_url(async_url) - driver = u.drivername - - if driver.startswith("sqlite+aiosqlite"): - u = u.set(drivername="sqlite") - elif driver.startswith("postgresql+asyncpg"): - u = u.set(drivername="postgresql") - else: - # Generic: strip the async driver part if present - if "+" in driver: - u = u.set(drivername=driver.split("+", 1)[0]) - - return str(u) - - def _get_sqlite_file_path(sync_url: str) -> Optional[str]: """Return the on-disk path for a SQLite URL, else None.""" try: @@ -159,7 +142,7 @@ async def init_db_engine() -> None: await conn.execute(text("PRAGMA foreign_keys = ON;")) await conn.execute(text("PRAGMA synchronous = NORMAL;")) - await _run_migrations(raw_url=db_url, connect_args=connect_args) + await _run_migrations(database_url=db_url, connect_args=connect_args) SESSION = async_sessionmaker( bind=ENGINE, @@ -170,20 +153,18 @@ async def init_db_engine() -> None: ) -async def _run_migrations(raw_url: str, connect_args: dict) -> None: - """ - Run Alembic migrations up to head. +async def _run_migrations(database_url: str, connect_args: dict) -> None: + if database_url.find("postgresql+psycopg") == -1: + """SQLite: Convert an async SQLAlchemy URL to a sync URL for Alembic.""" + u = make_url(database_url) + driver = u.drivername + if not driver.startswith("sqlite+aiosqlite"): + raise ValueError(f"Unsupported DB driver: {driver}") + database_url, is_mem = _normalize_sqlite_memory_url(str(u.set(drivername="sqlite"))) + database_url = _absolutize_sqlite_url(database_url) - We deliberately use a synchronous engine for migrations because Alembic's - programmatic API is synchronous by default and this path is robust. - """ - # Convert to sync URL and make SQLite URL an absolute one - sync_url = _to_sync_driver_url(raw_url) - sync_url, is_mem = _normalize_sqlite_memory_url(sync_url) - sync_url = _absolutize_sqlite_url(sync_url) - - cfg = _get_alembic_config(sync_url) - engine = create_engine(sync_url, future=True, connect_args=connect_args) + cfg = _get_alembic_config(database_url) + engine = create_engine(database_url, future=True, connect_args=connect_args) with engine.connect() as conn: context = MigrationContext.configure(conn) current_rev = context.get_current_revision() @@ -203,7 +184,7 @@ async def _run_migrations(raw_url: str, connect_args: dict) -> None: # Optional backup for SQLite file DBs backup_path = None - sqlite_path = _get_sqlite_file_path(sync_url) + sqlite_path = _get_sqlite_file_path(database_url) if sqlite_path and os.path.exists(sqlite_path): backup_path = sqlite_path + ".bkp" try: diff --git a/app/database/models.py b/app/database/models.py index 5bb3a09bc..55fc08e51 100644 --- a/app/database/models.py +++ b/app/database/models.py @@ -1,5 +1,3 @@ -from __future__ import annotations - from datetime import datetime from typing import Any, Optional import uuid @@ -18,11 +16,15 @@ from sqlalchemy import ( Numeric, Boolean, ) +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship, foreign from .timeutil import utcnow +JSONB_V = JSON(none_as_null=True).with_variant(JSONB(none_as_null=True), 'postgresql') + + class Base(DeclarativeBase): pass @@ -46,7 +48,7 @@ class Asset(Base): hash: Mapped[str] = mapped_column(String(256), primary_key=True) size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False, default=0) - mime_type: Mapped[str | None] = mapped_column(String(255)) + mime_type: Mapped[Optional[str]] = mapped_column(String(255)) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=False), nullable=False, default=utcnow ) @@ -97,7 +99,7 @@ class AssetCacheState(Base): id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) asset_hash: Mapped[str] = mapped_column(String(256), ForeignKey("assets.hash", ondelete="CASCADE"), nullable=False) file_path: Mapped[str] = mapped_column(Text, nullable=False) - mtime_ns: Mapped[int | None] = mapped_column(BigInteger, nullable=True) + mtime_ns: Mapped[Optional[int]] = mapped_column(BigInteger, nullable=True) asset: Mapped["Asset"] = relationship(back_populates="cache_states") @@ -122,9 +124,9 @@ class AssetLocation(Base): asset_hash: Mapped[str] = mapped_column(String(256), ForeignKey("assets.hash", ondelete="CASCADE"), nullable=False) provider: Mapped[str] = mapped_column(String(32), nullable=False) # "gcs" locator: Mapped[str] = mapped_column(Text, nullable=False) # "gs://bucket/object" - expected_size_bytes: Mapped[int | None] = mapped_column(BigInteger, nullable=True) - etag: Mapped[str | None] = mapped_column(String(256), nullable=True) - last_modified: Mapped[str | None] = mapped_column(String(128), nullable=True) + expected_size_bytes: Mapped[Optional[int]] = mapped_column(BigInteger, nullable=True) + etag: Mapped[Optional[str]] = mapped_column(String(256), nullable=True) + last_modified: Mapped[Optional[str]] = mapped_column(String(128), nullable=True) asset: Mapped["Asset"] = relationship(back_populates="locations") @@ -144,8 +146,8 @@ class AssetInfo(Base): asset_hash: Mapped[str] = mapped_column( String(256), ForeignKey("assets.hash", ondelete="RESTRICT"), nullable=False ) - preview_hash: Mapped[str | None] = mapped_column(String(256), ForeignKey("assets.hash", ondelete="SET NULL")) - user_metadata: Mapped[dict[str, Any] | None] = mapped_column(JSON(none_as_null=True)) + preview_hash: Mapped[Optional[str]] = mapped_column(String(256), ForeignKey("assets.hash", ondelete="SET NULL")) + user_metadata: Mapped[Optional[dict[str, Any]]] = mapped_column(JSON(none_as_null=True)) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=False), nullable=False, default=utcnow ) @@ -162,7 +164,7 @@ class AssetInfo(Base): back_populates="infos", foreign_keys=[asset_hash], ) - preview_asset: Mapped[Asset | None] = relationship( + preview_asset: Mapped[Optional[Asset]] = relationship( "Asset", back_populates="preview_of", foreign_keys=[preview_hash], @@ -220,7 +222,7 @@ class AssetInfoMeta(Base): val_str: Mapped[Optional[str]] = mapped_column(String(2048), nullable=True) val_num: Mapped[Optional[float]] = mapped_column(Numeric(38, 10), nullable=True) val_bool: Mapped[Optional[bool]] = mapped_column(Boolean, nullable=True) - val_json: Mapped[Optional[Any]] = mapped_column(JSON(none_as_null=True), nullable=True) + val_json: Mapped[Optional[Any]] = mapped_column(JSONB_V, nullable=True) asset_info: Mapped["AssetInfo"] = relationship(back_populates="metadata_entries") diff --git a/tests-assets/conftest.py b/tests-assets/conftest.py index 16a43d56d..88fc0a5f3 100644 --- a/tests-assets/conftest.py +++ b/tests-assets/conftest.py @@ -15,6 +15,22 @@ import pytest_asyncio import subprocess +def pytest_addoption(parser: pytest.Parser) -> None: + """ + Allow overriding the database URL used by the spawned ComfyUI process. + Priority: + 1) --db-url command line option + 2) ASSETS_TEST_DB_URL environment variable (used by CI) + 3) default: sqlite in-memory + """ + parser.addoption( + "--db-url", + action="store", + default=os.environ.get("ASSETS_TEST_DB_URL", "sqlite+aiosqlite:///:memory:"), + help="Async SQLAlchemy DB URL (e.g. sqlite+aiosqlite:///:memory: or postgresql+psycopg://user:pass@host/db)", + ) + + def _free_port() -> int: with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.bind(("127.0.0.1", 0)) @@ -49,30 +65,38 @@ def event_loop(): @pytest.fixture(scope="session") def comfy_tmp_base_dir() -> Path: - tmp = Path(tempfile.mkdtemp(prefix="comfyui-assets-tests-")) + env_base = os.environ.get("ASSETS_TEST_BASE_DIR") + created_by_fixture = False + if env_base: + tmp = Path(env_base) + tmp.mkdir(parents=True, exist_ok=True) + else: + tmp = Path(tempfile.mkdtemp(prefix="comfyui-assets-tests-")) + created_by_fixture = True _make_base_dirs(tmp) yield tmp - with contextlib.suppress(Exception): - for p in sorted(tmp.rglob("*"), reverse=True): - if p.is_file() or p.is_symlink(): - p.unlink(missing_ok=True) - for p in sorted(tmp.glob("**/*"), reverse=True): - with contextlib.suppress(Exception): - p.rmdir() - tmp.rmdir() + if created_by_fixture: + with contextlib.suppress(Exception): + for p in sorted(tmp.rglob("*"), reverse=True): + if p.is_file() or p.is_symlink(): + p.unlink(missing_ok=True) + for p in sorted(tmp.glob("**/*"), reverse=True): + with contextlib.suppress(Exception): + p.rmdir() + tmp.rmdir() @pytest.fixture(scope="session") -def comfy_url_and_proc(comfy_tmp_base_dir: Path): +def comfy_url_and_proc(comfy_tmp_base_dir: Path, request: pytest.FixtureRequest): """ Boot ComfyUI subprocess with: - sandbox base dir - - sqlite memory DB + - sqlite memory DB (default) - autoscan disabled Returns (base_url, process, port) """ port = _free_port() - db_url = "sqlite+aiosqlite:///:memory:" + db_url = request.config.getoption("--db-url") logs_dir = comfy_tmp_base_dir / "logs" logs_dir.mkdir(exist_ok=True) @@ -94,6 +118,7 @@ def comfy_url_and_proc(comfy_tmp_base_dir: Path): "127.0.0.1", "--port", str(port), + "--cpu", ], stdout=out_log, stderr=err_log, @@ -101,6 +126,13 @@ def comfy_url_and_proc(comfy_tmp_base_dir: Path): env={**os.environ}, ) + for _ in range(50): + if proc.poll() is not None: + out_log.flush() + err_log.flush() + raise RuntimeError(f"ComfyUI exited early with code {proc.returncode}") + time.sleep(0.1) + base_url = f"http://127.0.0.1:{port}" try: async def _probe(): @@ -113,6 +145,9 @@ def comfy_url_and_proc(comfy_tmp_base_dir: Path): with contextlib.suppress(Exception): proc.terminate() proc.wait(timeout=10) + with contextlib.suppress(Exception): + out_log.flush() + err_log.flush() raise RuntimeError(f"ComfyUI did not become ready: {e}") if proc and proc.poll() is None: @@ -144,7 +179,7 @@ async def _post_multipart_asset( tags: list[str], meta: dict, data: bytes, - extra_fields: dict | None = None, + extra_fields: Optional[dict] = None, ) -> tuple[int, dict]: form = aiohttp.FormData() form.add_field("file", data, filename=name, content_type="application/octet-stream")