From 12ff8a1955fa0affa724a356c5c676f71199e535 Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 29 Sep 2026 11:54:28 -0400 Subject: [PATCH] build: Drop Python 3.11 support and promote clodius to a core dependency The tiles extra existed only to keep 3.11 installable: clodius (matrix tile serving, issue #82) requires Python >= 3.12, so it carried a python_version marker and stayed optional rather than force the floor up. That meant every image built from the standard Dockerfiles -- including what actually deploys to dev/prod -- carried no tile backend at all, so the tile routes always answered 501 regardless of database content. Raise requires-python to >=3.12 and move clodius into dependencies unconditionally. Every image already builds on python:3.12-slim-bookworm, so this changes nothing about the runtime base -- it only drops the promise that a 3.11 interpreter can install cfdb, and both the API and worker images now carry the tile backend by default. Retires Dockerfile.api-tiles and the api-tiles/worker-tiles Makefile scaffolding that existed solely to work around clodius being optional locally, exactly as anticipated when that scaffolding was written -- `make api` and `make wool` cover it now. Drops the 3.11 leg from the CI test matrix, and removes the pytest.importorskip("clodius")/("numpy") guards added to keep the (now nonexistent) 3.11 CI leg passing; the sys.modules-patching tests that simulate a genuinely backendless build are untouched, since that degradation path remains real architecture, not a version-support artifact. --- .github/workflows/test.yml | 2 +- Dockerfile.api-tiles | 36 ------------ Makefile | 56 ------------------ README.md | 25 +++----- pyproject.toml | 66 +++++++++------------- src/cfdb/api/main.py | 10 ++-- src/cfdb/tilesets/backend.py | 18 +++--- src/cfdb/tilesets/errors.py | 8 +-- tests/fixtures/coolers.py | 7 +-- tests/integration/conftest.py | 9 +-- tests/integration/test_tiles_e2e.py | 12 ++-- tests/integration/test_tiles_s3_profile.py | 5 +- tests/test_api/test_lifespan_tilesets.py | 8 --- tests/test_tiles.py | 6 +- tests/test_tilesets/test_backend.py | 31 ++++------ tests/test_tilesets/test_service.py | 6 +- tests/test_tilesets_router.py | 13 ----- 17 files changed, 79 insertions(+), 239 deletions(-) delete mode 100644 Dockerfile.api-tiles diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 729157b..c07b480 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,7 +19,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.11", "3.12", "3.13"] + python-version: ["3.12", "3.13"] steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 diff --git a/Dockerfile.api-tiles b/Dockerfile.api-tiles deleted file mode 100644 index bb24df6..0000000 --- a/Dockerfile.api-tiles +++ /dev/null @@ -1,36 +0,0 @@ -# Local-development image: the API image plus the clodius tile backend. -# -# Interim scaffolding for issue #82. The clodius `tiles` extra is still -# optional (its fork requires Python 3.12 while cfdb supports 3.11), so -# `Dockerfile.api` does not carry it and the images it builds answer 501 -# on every tile route (see the `tiles` extra in pyproject.toml). That is -# the right production behaviour today, but it makes the tile endpoints -# untestable through the documented `make mongodb` / `make api` flow, -# which is the only place the whole chain -- workflow dispatch, artifact -# cache, h5py read -- runs together. -# -# So this layers clodius onto the image `make api` already built rather -# than duplicating that build, installing it straight from the public -# fork at the same sha `[tool.uv.sources]` pins. -# -# DELETE THIS FILE once the `tiles` extra becomes an ordinary dependency -# (which raises the Python floor to 3.12): `Dockerfile.api`'s existing -# `pip install .` then carries it. -FROM api - -USER root - -# Installed as its own layer, before the app, so that iterating on cfdb -# does not re-resolve clodius's 25-package closure (pysam, oxbow, apsw, -# polars, scipy, ...) on every rebuild. Keep this sha in lockstep with -# the `[tool.uv.sources]` pin in pyproject.toml. -RUN pip install --no-cache-dir \ - "clodius @ git+https://github.com/abdenlab/clodius@aac90219031e8c05cf6d5a916628531a72cfc461" - -# clodius imports matplotlib transitively, and the image's `app` user -# has no home directory, so matplotlib warns twice and rebuilds its font -# cache on every start. Point it at a writable path. -ENV MPLCONFIGDIR=/tmp/matplotlib - -USER app -WORKDIR /app/src diff --git a/Makefile b/Makefile index dbf0211..a163976 100644 --- a/Makefile +++ b/Makefile @@ -52,62 +52,6 @@ api: docker run -d --name api --network cvh-backend-network --network-alias cvh-backend -p 8000:8000 -e SYNC_DATA_DIR=/tmp/sync-data api @echo "API container is up and running on port 8000 (http://0.0.0.0:8000/metadata)." -# --- Local-dev matrix tile serving (issue #82) ----------------------- -# -# `make api` builds an image with no clodius, so its tile routes answer -# 501 -- correct for production while the `tiles` extra stays optional, -# but it leaves the whole tile chain untestable through the documented -# Docker flow. These targets layer clodius on (installed from the public -# fork at the sha pyproject.toml pins) and add the piece `make api` does -# not need: a cache volume shared with a worker, since a tileset -# artifact is built by the worker and then opened by the API. -# -# Retire both once `tiles` becomes an ordinary dependency (raising the -# Python floor to 3.12) -- `make api` then covers this on its own. - -api-tiles: - make network - @docker stop api 2>/dev/null || true - @docker rm api 2>/dev/null || true - @echo "Building the API image..." - docker build -t api -f Dockerfile.api . - @echo "Layering the clodius tile backend on top..." - docker build -t api-tiles -f Dockerfile.api-tiles . - @# The image runs as `app`, but a freshly created named volume is - @# owned by root, so the lifespan's mkdir under SYNC_DATA_DIR fails - @# with PermissionError and the API 500s on every workflow route. - @docker volume create cfdb-sync >/dev/null - @docker run --rm --user 0 --entrypoint chown -v cfdb-sync:/tmp/sync-data api-tiles -R app:app /tmp/sync-data - @echo "Starting the API container with matrix tile serving enabled..." - docker run -d --name api --network cvh-backend-network --network-alias cvh-backend -p 8000:8000 \ - -e SYNC_DATA_DIR=/tmp/sync-data \ - -e WORKFLOW_POOL_NAMESPACE=$${WORKFLOW_POOL_NAMESPACE:-cfdb-workers} \ - -v cfdb-sync:/tmp/sync-data \ - api-tiles - @echo "API up on port 8000." - @# Start the worker FIRST next time: the API dispatches on request, and a - @# dispatch that finds no worker is queued rather than failed, so the job - @# then waits out CFDB_WORKFLOW_RETRY_INTERVAL_S (2 min) before running. - @echo "Run 'make worker-tiles' before POSTing a tileset, or the first job waits ~2 min for the retry tick." - -# The worker that builds tileset artifacts. It reuses the API image -# rather than `cfdb-wool`, because the matrix processor needs cooler and -# h5py, which arrive with clodius and are absent from the worker image. -# It shares the API's cache volume: the worker writes the artifact, the -# API opens it locally with h5py. -worker-tiles: - make network - @docker stop worker 2>/dev/null || true - @docker rm worker 2>/dev/null || true - @echo "Starting a containerized LAN worker pool (namespace=$${WORKFLOW_POOL_NAMESPACE:-cfdb-workers})..." - docker run -d --name worker --network cvh-backend-network \ - -e SYNC_DATA_DIR=/tmp/sync-data \ - -e WORKFLOW_POOL_NAMESPACE=$${WORKFLOW_POOL_NAMESPACE:-cfdb-workers} \ - -e WORKFLOW_WORKER_COUNT=$${WORKFLOW_WORKER_COUNT:-1} \ - -v cfdb-sync:/tmp/sync-data \ - api-tiles python -m cfdb.workflows.worker_lan - @echo "Worker container is up. Check logs with: docker logs -f worker" - schema: @echo "Regenerating schema.graphql from the Strawberry schema..." uv run python scripts/export_schema.py diff --git a/README.md b/README.md index 6001dae..a85b006 100644 --- a/README.md +++ b/README.md @@ -10,15 +10,9 @@ CFDB is a Python package for querying and serving enriched C2M2 (Crosscut Metada pip install git+https://github.com/abdenlab/cfdb.git ``` -Requires Python 3.11 or later. +Requires Python 3.12 or later. -Matrix tile serving for Hi-C contact maps is an optional extra, because the [`abdenlab/clodius`](https://github.com/abdenlab/clodius) fork it depends on requires Python 3.12 while cfdb still supports 3.11. The dependency is pinned to a commit sha of the public fork, so enabling it needs nothing beyond the extra: - -```bash -uv sync --extra tiles -``` - -Without it the API runs unchanged and the tile endpoints answer `501`. See [Matrix tile serving](#matrix-tile-serving-hi-c-contact-maps). +Matrix tile serving for Hi-C contact maps (see [Matrix tile serving](#matrix-tile-serving-hi-c-contact-maps)) is backed by the [`abdenlab/clodius`](https://github.com/abdenlab/clodius) fork, pinned to a commit sha of the public fork as an ordinary dependency — no separate install step is needed beyond the usual `pip install` / `uv sync` above. ### Environment Variables @@ -930,7 +924,7 @@ Both endpoints accept repeatable `d` parameters, capped at 64 per request. The r | 400 | Malformed uid, no `d` parameter, more than 64 `d` parameters, or an unknown DCC | | 403 | File requires consortium/protected access (HuBMAP) | | 404 | File not found, not a contact map, or its tileset artifact has not been built yet | -| 501 | This build carries no tile backend (see the `tiles` extra below), or the file is a `.hic` | +| 501 | This build carries no tile backend (a broken or incomplete install), or the file is a `.hic` | | 503 | Workflow subsystem disabled, or the artifact could not be pulled onto local disk in time (`Retry-After`) | #### Preparing a contact map @@ -958,16 +952,16 @@ curl http://localhost:8000/tilesets/4dn//status # {"ready": true} #### Serving tiles locally -`make api` builds from `Dockerfile.api`, which carries no clodius, so its tile routes answer `501` — correct while the `tiles` extra stays optional, but it leaves the whole chain untestable through the documented Docker flow. `make api-tiles` layers the tile backend on (installed from the public fork at the same sha `pyproject.toml` pins) and adds the one piece `make api` does not need: a cache volume shared with a worker, because a tileset artifact is built by the worker and then opened locally by the API with h5py. Both targets are interim scaffolding and are retired once `tiles` becomes an ordinary dependency, at which point `make api` covers this on its own. - -The worker reuses the API image rather than `cfdb-wool`: the matrix processor needs `cooler` and `h5py`, which arrive with clodius and are absent from the worker image. +`clodius` is an ordinary dependency (see [Installation](#installation)), so `make api` and `make wool` both carry the tile backend directly — there is no separate tile-enabled image to build or retire. Start MongoDB, a worker, and the API the same way any other workflow-subsystem feature is tested locally: ```bash make mongodb # database -make worker-tiles # worker FIRST — see below -make api-tiles # API with the tile backend +make worker-local # worker FIRST — see below +uv run uvicorn cfdb.api.main:app --reload # API, with SYNC_DATA_DIR set to the same path the worker uses ``` +`make api` remains available for a quick container-only smoke test, but a container's `SYNC_DATA_DIR` is private to that container's filesystem — running the API and a worker in separate containers needs a shared bind mount for the cache to work, same as any other `SYNC_DATA_DIR`-dependent flow (see [Running a local worker pool](#running-a-local-worker-pool) below). Running both as bare `uv run` processes against the same local `SYNC_DATA_DIR` sidesteps that entirely. + Start the worker **before** POSTing a tileset. A dispatch that finds no worker is queued rather than failed, so the job then waits out `CFDB_WORKFLOW_RETRY_INTERVAL_S` (2 min) before the retry tick picks it up — correct behaviour, but a confusing two minutes of `pending` if you were not expecting it. The database starts empty, so seed a contact-map document before requesting anything: `/tileset_info` and `/tiles` resolve a file by `{submission, local_id}` and need `filename` (the `.mcool` suffix is what routes it), `md5` (the cache key is content-addressed on it), `access_url`, `file_format.name: "HDF5"`, `dcc.dcc_abbreviation`, and `data_access_level: "public"`, plus `genome_assembly` if you want `coordSystem` in the payload. Note that 4DN's `@@download` URLs answer `403` without credentials; use the open-data S3 mirror (`4dn-open-data-public.s3.amazonaws.com`), which the URL allowlist already covers. @@ -997,8 +991,7 @@ This is invisible to the client, for two reasons that were verified rather than #### Limitations -- **The clodius tile backend is an optional extra, so deployed images do not carry it.** Tile reading is backed by the [`abdenlab/clodius`](https://github.com/abdenlab/clodius) fork's `tiles_v2` subpackage, installed via `uv sync --extra tiles` from a commit-pinned git reference on the fork's `main` (a sha, deliberately, rather than a floating branch reference — the fork treats `tiles_v2` as unstable and cfdb is its first consumer, so what runs must be exactly what was validated). The extra stays optional because clodius requires Python 3.12 while cfdb still supports 3.11, so `Dockerfile.api` and `Dockerfile.wool` do not install it and images built today answer `501` on the tile routes. Promoting the dependency to an unconditional one — which raises the floor to 3.12 and lets the images carry the backend — is tracked as follow-up work. -- **ENCODE `.hic` contact maps are not served.** The pinned clodius does not carry a `.hic` tileset at all, which makes this an unconditional `501` today. It would stay unconditional even if a future clodius did carry one: `.hic` support (tracked separately, on a different path than the tiles_v2 backend above) is built on `hictkpy`, whose entry points take `str | os.PathLike` and nothing else. There is no URL, HTTP, or S3 surface in the library, so a `.hic` can only be read from a local copy. Making that copy is not viable at this corpus's scale: 3,903 files totalling ~78 TB, a median of 10.3 GB, and a largest single file of 315 GB — past the 200 GiB ceiling on Fargate ephemeral storage, so it could not be cached on an API task at any budget. Serving them needs either a remote-capable reader (hictk tracks S3 support in [paulsengroup/hictk#395](https://github.com/paulsengroup/hictk/issues/395), currently scoped to Cooler rather than `.hic`) or an explicit size cap accepting partial coverage — an 8 GiB cap would reach 43% of the files, 20 GiB 67%, 50 GiB 91%. That is deferred rather than decided. 4DN `.mcool` files are unaffected. +- **ENCODE `.hic` contact maps are not served.** The pinned clodius does not carry a `.hic` tileset at all, which makes this an unconditional `501` today. It would stay unconditional even if a future clodius did carry one: `.hic` support is built on `hictkpy`, whose entry points take `str | os.PathLike` and nothing else. There is no URL, HTTP, or S3 surface in the library, so a `.hic` can only be read from a local copy. Making that copy is not viable at this corpus's scale: 3,903 files totalling ~78 TB, a median of 10.3 GB, and a largest single file of 315 GB — past the 200 GiB ceiling on Fargate ephemeral storage, so it could not be cached on an API task at any budget. Serving them needs either a remote-capable reader (hictk tracks S3 support in [paulsengroup/hictk#395](https://github.com/paulsengroup/hictk/issues/395), currently scoped to Cooler rather than `.hic`) or an explicit size cap accepting partial coverage — an 8 GiB cap would reach 43% of the files, 20 GiB 67%, 50 GiB 91%. That is deferred rather than decided. 4DN `.mcool` files are unaffected. - **`coordSystem` is omitted for the 21 4DN contact maps that carry no `genomeAssembly`.** It is left out entirely rather than emitted as an empty string: HiGlass matches `coordSystem` against chromosome-info tilesets, so a plausible-looking placeholder risks a silently misaligned track, whereas an absent key makes the client fall back to the `chromsizes` array every tileset info carries. The heatmap still renders. - **ICE-balanced heatmaps are unavailable for the 10 flat `.cool` files.** Coarsening runs without `--balance` because balancing a large flat cooler costs minutes to hours; clodius falls back cleanly to unbalanced when no `weight` column exists. - **Interchromosomal tiles are served as the concatenated-genome lattice clodius produces**, with no special-casing. diff --git a/pyproject.toml b/pyproject.toml index 2a2945c..329cd03 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,13 +5,21 @@ requires = ["setuptools>=64", "setuptools-scm>=8"] [project] authors = [{ name = "Conrad Bzura", email = "conradbzura@gmail.com" }] classifiers = [ - "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", ] dependencies = [ "aiohttp", "boto3", + # Matrix tile serving (issue #82). Was an optional extra gated + # `python_version >= '3.12'` because the fork requires 3.12 while + # cfdb still supported 3.11; now that the floor is 3.12, it is an + # ordinary dependency like any other. `clodius` on public PyPI is a + # different codebase from the fork this consumes — the public + # releases carry no `tiles_v2` — which is why [tool.uv.sources] + # below still pins it to a commit sha on the fork rather than + # resolving the name from PyPI. + "clodius", "click", "debugpy", "fastapi", @@ -36,33 +44,16 @@ dynamic = ["version"] license = { text = "MIT License" } name = "cfdb" readme = { file = "README.md", content-type = "text/markdown" } -requires-python = ">=3.11" +# Raised from >=3.11 to >=3.12 to carry clodius (issue #82's matrix tile +# backend) as an unconditional dependency instead of an optional extra — +# see the `dependencies` comment above. Every image already builds on +# python:3.12-slim-bookworm, so this does not change the runtime base; +# it only drops the promise that a 3.11 interpreter can install cfdb at +# all. +requires-python = ">=3.12" [project.optional-dependencies] -dev = ["allpairspy", "clodius; python_version >= '3.12'", "cloudpickle", "cryptography", "debugpy", "httpx", "hypothesis", "mongomock-motor", "moto[ecs,s3]", "pytest-asyncio", "pytest-mock", "ruff"] -# Matrix tile serving (issue #82). Deliberately an extra rather than a -# runtime dependency, for two reasons that both bite if it moves. -# -# First, `clodius` on public PyPI is a different codebase from the fork -# this consumes — the public releases carry no ``tiles_v2`` — so a bare -# `pip install .` that resolved the name from PyPI would install a -# package that imports and then fails at the first tile. An extra makes -# that resolution impossible rather than merely unlikely. -# -# Second, the fork requires Python >= 3.12 while cfdb still supports -# 3.11. An extra alone does not buy that back — uv resolves universally -# across the whole `requires-python` range and rejects the lock outright -# — so the dependency carries an explicit `python_version >= '3.12'` -# marker. On 3.11 the requirement simply is not there, which is what lets -# the 3.11 support promise survive until the dependency is made -# unconditional. -# -# The consequence is that images built from the Dockerfiles do NOT carry -# clodius, so the tile routes answer 501 there. See ``cfdb.tilesets.backend`` -# for the single import gate that makes that degradation explicit rather -# than a partial failure. Promoting this to a real dependency is tracked -# as follow-up work, gated on the fork being published. -tiles = ["clodius; python_version >= '3.12'"] +dev = ["allpairspy", "cloudpickle", "cryptography", "debugpy", "httpx", "hypothesis", "mongomock-motor", "moto[ecs,s3]", "pytest-asyncio", "pytest-mock", "ruff"] [tool.uv.sources] # Pinned to a sha, not a branch: clodius considers ``tiles_v2`` unstable @@ -72,22 +63,17 @@ tiles = ["clodius; python_version >= '3.12'"] # guards, see ``cfdb.tilesets.backend.ClodiusBackend.tile_error``, now # bound to ``TilesetError``, and the tile-omission contract change # documented on ``cfdb.tilesets.service.TilesetService._read_tiles``) has -# been merged there. Even though ``main`` now carries ``tiles_v2``, stay -# pinned to a commit sha rather than a branch reference — a branch -# reference floats, and what runs must be exactly what was validated. +# been merged there. Stay pinned to a commit sha rather than a branch +# reference regardless — a branch reference floats, and what runs must +# be exactly what was validated. # # ``main`` does not carry ``clodius.tiles_v2.hic`` — ``load_hic_tileset`` -# degrades to ``TileBackendUnavailable`` for it, same as a build with no -# clodius at all. That's a no-op for cfdb today: ``.hic`` is refused at -# the call site regardless (see ``TilesetService._open_hic``) because the -# module reads local files only, which does not fit this corpus at any -# disk budget. Serving ``.hic`` at all is tracked as separate follow-up -# work, not something this pin is expected to grow. -# -# The dependency stays an optional extra rather than becoming -# unconditional because the fork requires Python >= 3.12 while cfdb -# still supports 3.11; promoting it (and raising the floor) is tracked -# as follow-up. +# degrades to ``TileBackendUnavailable`` for it. That's a no-op for cfdb +# today: ``.hic`` is refused at the call site regardless (see +# ``TilesetService._open_hic``) because the module reads local files +# only, which does not fit this corpus at any disk budget. Serving +# ``.hic`` at all is tracked as separate follow-up work, not something +# this pin is expected to grow. clodius = { git = "https://github.com/abdenlab/clodius", rev = "07f043bea3380bbc38e76f0970d93abed0be4f69" } [project.scripts] diff --git a/src/cfdb/api/main.py b/src/cfdb/api/main.py index 7e76dab..bcb4dc7 100644 --- a/src/cfdb/api/main.py +++ b/src/cfdb/api/main.py @@ -166,11 +166,11 @@ def _build_tileset_service( ) -> "TilesetService | None": """Build the matrix tile service, or explain why there isn't one. - Returns ``None`` when the installed build carries no clodius. That is - the expected state for images built today — the fork is private, so - the Dockerfiles cannot fetch it and it is installed only via the - ``tiles`` extra — and it must degrade to a clear 501 on the tile - routes rather than taking the whole application down at startup. + Returns ``None`` when the installed build carries no clodius. clodius + is an ordinary dependency, so this is a broken or incomplete install + rather than an expected state — but it must still degrade to a clear + 501 on the tile routes rather than taking the whole application down + at startup. The service reads ``api.cache`` through a callable rather than capturing it, so a lifespan that rebuilds the cache (or a test that diff --git a/src/cfdb/tilesets/backend.py b/src/cfdb/tilesets/backend.py index 4f88896..5f35cab 100644 --- a/src/cfdb/tilesets/backend.py +++ b/src/cfdb/tilesets/backend.py @@ -1,12 +1,12 @@ """The single import gate for the clodius tile backend. Every import of clodius in cfdb goes through here, so the whole subsystem -has exactly one on/off seam. That matters because clodius is currently an -optional extra installed from a sibling checkout (the fork is private, so -the Dockerfiles cannot fetch it) — which means production images do not -carry it. Concentrating the import lets that degrade to a clear 501 on the -tile routes instead of an ImportError at application startup or, worse, a -subsystem that half-works. +has exactly one on/off seam. clodius is an ordinary dependency (see +``pyproject.toml``) and every image carries it, so in practice this gate +never trips — but concentrating the import here means a broken or +partial install still degrades to a clear 501 on the tile routes instead +of an ImportError at application startup, or worse, a subsystem that +half-works. The gate also resolves ``clodius.tiles_v2.hic`` separately from the cooler tileset. The module is not part of the pinned build — cfdb does not serve @@ -61,9 +61,9 @@ def load_backend() -> ClodiusBackend: except ImportError as exc: # pragma: no cover - exercised via monkeypatch raise TileBackendUnavailable( "Matrix tile serving requires the clodius tile backend, which " - "this build does not carry. Install the 'tiles' extra " - "(`uv sync --extra tiles`, which resolves clodius from the " - "sibling checkout) to enable it." + "this build does not carry. clodius is an ordinary dependency " + "(`uv sync`), so this means the install is broken or " + "incomplete rather than a feature that needs enabling." ) from exc return ClodiusBackend( diff --git a/src/cfdb/tilesets/errors.py b/src/cfdb/tilesets/errors.py index e4a0245..fab4a56 100644 --- a/src/cfdb/tilesets/errors.py +++ b/src/cfdb/tilesets/errors.py @@ -42,10 +42,10 @@ class TileBackendUnavailable(TilesetError): """The installed build carries no usable tile backend. Rendered as 501. Two distinct causes share it: clodius is absent - entirely (the images do not yet carry it — see the ``tiles`` extra in - ``pyproject.toml``), or the request is for a ``.hic``, which cfdb - deliberately does not serve because the backend reads local files only - and those files are too large to cache. + entirely (a broken or incomplete install — it is an ordinary + dependency, see ``pyproject.toml``), or the request is for a ``.hic``, + which cfdb deliberately does not serve because the backend reads + local files only and those files are too large to cache. """ diff --git a/tests/fixtures/coolers.py b/tests/fixtures/coolers.py index ed5e82e..cd3ef96 100644 --- a/tests/fixtures/coolers.py +++ b/tests/fixtures/coolers.py @@ -23,14 +23,9 @@ from pathlib import Path +import numpy as np import pytest -#: numpy arrives transitively with clodius/cooler, which are only -#: installed for the ``tiles`` extra (Python >= 3.12). Guard here rather -#: than let a bare ``import numpy`` fail collection outright on 3.11, -#: since every caller of this module already expects a skip in that case. -np = pytest.importorskip("numpy") - #: Tiling genome, 3000 bp total. Deliberately tiny relative to the bin #: sizes below: a cooler tile is 256 bins wide, so a realistic 1 kb binsize #: on this genome would put the whole thing inside a single bin of a single diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 126beb3..bfeba9e 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -103,10 +103,11 @@ def tool_available(name: str) -> bool: def cooler_available() -> bool: """Return True when the ``cooler`` package is importable. - The tiles extra is optional (the clodius fork is private), so the - contact-map rows are gated the way the gffread/bedToBigBed rows are - gated on PATH tools — probed with ``find_spec`` so collection never - pays the import. + clodius is an ordinary dependency, so this is always True in a + correctly installed environment — kept as a probe (find_spec, so + collection never pays the import) for the same defense-in-depth + reason the gffread/bedToBigBed rows are gated on PATH tools: a + broken or partial install degrades to a skip rather than a failure. """ return importlib.util.find_spec("cooler") is not None diff --git a/tests/integration/test_tiles_e2e.py b/tests/integration/test_tiles_e2e.py index db16a04..6cc0647 100644 --- a/tests/integration/test_tiles_e2e.py +++ b/tests/integration/test_tiles_e2e.py @@ -16,17 +16,13 @@ import asyncio import base64 +import numpy as np import pytest import pytest_asyncio -# numpy arrives transitively with clodius/cooler, which are only installed -# for the tiles extra (Python >= 3.12). tests.fixtures.coolers guards its -# own numpy import the same way, but this module's own bare import would -# fail collection before ever reaching that one, so it needs the same -# guard directly. The tile backend itself loads lazily, so nothing else -# below pulls in cooler or clodius at import time -- guard the whole -# module here instead. -np = pytest.importorskip("numpy") +# The tile backend itself loads lazily, so nothing below pulls in cooler +# or clodius at import time -- guard the whole module here instead of +# letting the failure land inside the first fixture as an error. pytest.importorskip("cooler") pytest.importorskip("clodius.tiles_v2.cooler") diff --git a/tests/integration/test_tiles_s3_profile.py b/tests/integration/test_tiles_s3_profile.py index aa3777c..8337fdc 100644 --- a/tests/integration/test_tiles_s3_profile.py +++ b/tests/integration/test_tiles_s3_profile.py @@ -19,13 +19,10 @@ import asyncio import base64 +import numpy as np import pytest import pytest_asyncio -# numpy arrives transitively with clodius/cooler, which are only installed -# for the tiles extra (Python >= 3.12); guard it alongside the others -# rather than let a bare import fail collection outright on 3.11. -np = pytest.importorskip("numpy") pytest.importorskip("cooler") pytest.importorskip("moto") diff --git a/tests/test_api/test_lifespan_tilesets.py b/tests/test_api/test_lifespan_tilesets.py index 77d65c9..89594d4 100644 --- a/tests/test_api/test_lifespan_tilesets.py +++ b/tests/test_api/test_lifespan_tilesets.py @@ -79,9 +79,6 @@ async def test_lifespan_should_build_the_tile_service_when_clodius_is_present( ``api.tileset_service`` should be populated, so the tile routes serve rather than answering 501. """ - # Arrange - pytest.importorskip("clodius") - # Act async with main.lifespan(main.app): service = api.tileset_service @@ -133,7 +130,6 @@ async def test_lifespan_should_close_and_clear_the_tile_service_on_teardown( leak ``_reset_api_globals`` exists to prevent. """ # Arrange - pytest.importorskip("clodius") closed: list[bool] = [] # Act @@ -223,7 +219,6 @@ async def test_lifespan_should_propagate_config_to_the_tile_service( ``CFDB_TILESET_*`` / ``CFDB_TILE_*`` env knobs. """ # Arrange - pytest.importorskip("clodius") spy = mocker.spy(main, "build_tileset_service") # Act @@ -259,7 +254,6 @@ async def test_lifespan_should_null_all_globals_when_service_close_raises( the next app instantiation. """ # Arrange - pytest.importorskip("clodius") original_close = None # Act @@ -302,7 +296,6 @@ async def test_lifespan_should_build_the_tile_service_when_the_cache_is_s3( tiles are served. """ # Arrange - pytest.importorskip("clodius") assert stub_lifespan.kind == "s3-cached" # Act @@ -332,7 +325,6 @@ async def test_lifespan_should_wire_the_cache_provider_to_late_bind( service pointed at a stale backend. """ # Arrange - pytest.importorskip("clodius") from cfdb.tilesets.errors import TilesetNotReady doc = { diff --git a/tests/test_tiles.py b/tests/test_tiles.py index d60c49e..2e2560f 100644 --- a/tests/test_tiles.py +++ b/tests/test_tiles.py @@ -6,16 +6,12 @@ import base64 import itertools +import numpy as np import pytest from fastapi import HTTPException from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st -# numpy arrives transitively with clodius, which is only installed for the -# tiles extra (Python >= 3.12). Guard rather than let a bare import fail -# collection outright on 3.11. -np = pytest.importorskip("numpy") - from cfdb import api from cfdb.api.routers._helpers import PATH_PARAM_MAX_LEN from cfdb.api.routers.tiles import MAX_TILE_IDS, tiles, tileset_info diff --git a/tests/test_tilesets/test_backend.py b/tests/test_tilesets/test_backend.py index f9ee5b2..02a3131 100644 --- a/tests/test_tilesets/test_backend.py +++ b/tests/test_tilesets/test_backend.py @@ -3,7 +3,6 @@ from __future__ import annotations import sys -from importlib.util import find_spec import pytest @@ -29,23 +28,26 @@ class TestLoadBackend: """The one seam that turns the tile subsystem on or off.""" def test_should_raise_when_clodius_is_absent(self, mocker): - """Test the degraded import gate of a build without the tiles extra. + """Test the degraded import gate of a broken or incomplete install. Given: ``sys.modules`` patched so both clodius modules the gate - imports from raise ImportError, simulating an image built - without the ``tiles`` extra. + imports from raise ImportError, simulating a build whose + install of the ordinary clodius dependency is broken or + incomplete. When: load_backend is called. Then: - It should raise TileBackendUnavailable naming the ``tiles`` - extra, chained from the underlying ImportError. + It should raise TileBackendUnavailable, chained from the + underlying ImportError. """ # Arrange mocker.patch.dict(sys.modules, _ABSENT_BACKEND_MODULES) # Act & assert - with pytest.raises(TileBackendUnavailable, match="'tiles' extra") as excinfo: + with pytest.raises( + TileBackendUnavailable, match="clodius tile backend" + ) as excinfo: load_backend() assert isinstance(excinfo.value.__cause__, ImportError) @@ -62,7 +64,6 @@ def test_should_resolve_the_clodius_names(self): clodius exception types the per-tile/500 split keys on. """ # Arrange - pytest.importorskip("clodius") from clodius.core.errors import TileOutOfBounds, TilesetError from clodius.tiles_v2.cooler import CoolerTileset @@ -111,9 +112,6 @@ def test_should_return_true_when_clodius_is_present(self): Then: It should return True. """ - # Arrange - pytest.importorskip("clodius") - # Act result = is_available() @@ -151,16 +149,11 @@ def test_should_raise_when_the_hic_module_is_missing(self, mocker): class TestIsTileError: """The classification that splits per-tile payloads from 500s. - Every test here loads a real backend, so the whole class needs clodius - installed — unlike TestLoadBackend's absent-module tests, there is no - sys.modules trick that stands in for a real error hierarchy to - classify against. + Every test here loads a real backend and classifies against clodius's + real error hierarchy — unlike TestLoadBackend's absent-module tests, + there is no sys.modules trick standing in for it. """ - pytestmark = pytest.mark.skipif( - find_spec("clodius") is None, reason="requires the clodius tile backend" - ) - def test_should_recognize_a_clodius_tile_error(self): """Test that a clodius per-tile error is classified as such. diff --git a/tests/test_tilesets/test_service.py b/tests/test_tilesets/test_service.py index 480bcb2..5dacc31 100644 --- a/tests/test_tilesets/test_service.py +++ b/tests/test_tilesets/test_service.py @@ -5,15 +5,11 @@ import asyncio import base64 +import numpy as np import pytest from hypothesis import HealthCheck, example, given, settings from hypothesis import strategies as st -# numpy arrives transitively with clodius, which is only installed for the -# tiles extra (Python >= 3.12). Guard rather than let a bare import fail -# collection outright on 3.11. -np = pytest.importorskip("numpy") - from cfdb.tilesets.errors import ( TilesetHydrationTimeout, TilesetIdentityIncomplete, diff --git a/tests/test_tilesets_router.py b/tests/test_tilesets_router.py index 1a64f6e..9772045 100644 --- a/tests/test_tilesets_router.py +++ b/tests/test_tilesets_router.py @@ -105,7 +105,6 @@ async def test_should_report_not_ready_when_no_artifact_is_cached( never dispatching. """ # Arrange - pytest.importorskip("clodius") _, mock_db, executor, _ = prep_env mock_db.file.docs = [_file_doc()] @@ -233,7 +232,6 @@ async def test_should_503_when_the_workflow_subsystem_is_disabled( signals the same condition. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object(api, "cache", None) @@ -257,7 +255,6 @@ async def test_should_409_a_document_with_no_md5(self, prep_env): retry will fix and which is not the server's fault. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, _ = prep_env doc = _file_doc() del doc["md5"] @@ -313,7 +310,6 @@ async def test_should_503_a_hic_when_the_workflow_subsystem_is_disabled( reordering the guards would silently change the answer. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [ _file_doc(submission="encode", local_id="ENCFF123ABC", filename="c.hic") @@ -342,7 +338,6 @@ async def test_should_500_with_a_generic_detail_when_the_probe_raises( the exception text to an unauthenticated caller. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object( @@ -375,7 +370,6 @@ async def test_should_dispatch_and_point_at_the_job_endpoint(self, prep_env): for /data and /index preprocessing. """ # Arrange - pytest.importorskip("clodius") _, mock_db, executor, _ = prep_env mock_db.file.docs = [_file_doc()] @@ -460,7 +454,6 @@ async def test_should_429_when_the_admission_ceiling_is_reached( /index rather than queuing past it. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object( @@ -492,7 +485,6 @@ async def test_should_503_while_the_executor_is_draining(self, prep_env): a reorder would silently break. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object( @@ -517,7 +509,6 @@ async def test_should_409_when_the_executor_declines_the_file(self, prep_env): this file, not a server fault. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object( @@ -572,7 +563,6 @@ async def test_should_409_a_document_with_no_md5_without_dispatching( output could never be addressed must not be queued. """ # Arrange - pytest.importorskip("clodius") _, mock_db, executor, _ = prep_env doc = _file_doc() del doc["md5"] @@ -601,7 +591,6 @@ async def test_should_503_when_the_workflow_subsystem_is_disabled( build. """ # Arrange - pytest.importorskip("clodius") _, mock_db, executor, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object(api, "cache", None) @@ -629,7 +618,6 @@ async def test_should_500_with_a_generic_detail_when_the_probe_raises( the exception text to an unauthenticated caller. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object( @@ -662,7 +650,6 @@ async def test_should_500_with_a_generic_detail_when_the_executor_raises( it up as a 409 or 503 a client would retry forever. """ # Arrange - pytest.importorskip("clodius") _, mock_db, _, mocker = prep_env mock_db.file.docs = [_file_doc()] mocker.patch.object(