diff --git a/.claude/rules/database.md b/.claude/rules/database.md new file mode 100644 index 000000000..74f29ab80 --- /dev/null +++ b/.claude/rules/database.md @@ -0,0 +1,14 @@ +--- +paths: + - "src/mavedb/models/**" + - "src/mavedb/lib/**" + - "src/mavedb/routers/**" + - "src/mavedb/worker/**" + - "src/mavedb/scripts/**" +--- + +# Database + +- Write new queries in SQLAlchemy 2.0 `select()` style. Older code still uses `db.query()`; don't copy it. +- ValidTime tables never return only live rows by default. Filter with `Model.current` or `Model.live_at(...)`. +- Replace ValidTime rows with `supersede_with` or `supersede_live_where`, never a separate retire-then-insert, so the handoff has no gap. diff --git a/.claude/rules/migrations.md b/.claude/rules/migrations.md new file mode 100644 index 000000000..f9111b76f --- /dev/null +++ b/.claude/rules/migrations.md @@ -0,0 +1,13 @@ +--- +paths: + - "alembic/**" +--- + +# Migrations + +- Generate with `poetry run alembic revision --autogenerate -m ""`, then read the result: autogenerate misses or mangles enum changes, server defaults, views and functions. +- Set `down_revision` to the current head so the chain stays linear. Check with `poetry run alembic heads` (one head). +- Never edit a revision that has run on staging or production. Write a new one, even for a fix. +- One logical change per revision. The message says what changes ("add variant_count to scoresets"), not "update model". +- Production tables are large (`variants`, `mapping_records`, `mapping_record_alleles`, `alleles`, `annotation_event` run to millions of rows). A full-table `UPDATE` holds its lock for minutes. Build indexes on these tables `CONCURRENTLY` inside `op.get_context().autocommit_block()`, as `a7c3e5f9b1d2_unique_allele_hgvs_expression.py` does. +- Data migrations with no clean upgrade/downgrade path go in `alembic/manual_migrations/`. An operator runs them by hand (see the README there); Ruff excludes the directory. diff --git a/.claude/rules/permissions.md b/.claude/rules/permissions.md new file mode 100644 index 000000000..21f6b2221 --- /dev/null +++ b/.claude/rules/permissions.md @@ -0,0 +1,12 @@ +--- +paths: + - "src/mavedb/routers/**" + - "src/mavedb/lib/**" +--- + +# Permissions + +- Import from `mavedb.lib.permissions`. Check the entity a route names with `assert_permission(user, entity, Action.X)`. +- When a read fans out from that entity to related ones (score set → calibrations, allele → score sets), get a `Principal` from the `get_principal` dependency and filter with its `Viewer` for that entity type. Some entities are stricter than their parent: for instance, reading a score set doesn't grant reading its calibrations. Never assume that permission on one entity implies permission on its related entities. +- `Principal` and `Viewer` are request-scoped. Never use one as a default argument. +- Read enforcement is moving into Postgres row-level security (#808). Until it lands, these application checks are the only enforcement. diff --git a/.claude/rules/routes.md b/.claude/rules/routes.md new file mode 100644 index 000000000..5ae884830 --- /dev/null +++ b/.claude/rules/routes.md @@ -0,0 +1,15 @@ +--- +paths: + - "src/mavedb/routers/**" + - "src/mavedb/lib/authentication.py" + - "src/mavedb/lib/authorization.py" +--- + +# Routes + +FastAPI runs `def` handlers and dependencies in a thread pool and `async def` ones on the event loop, so a synchronous database call inside `async def` stalls every request on that process (#881). + +- New routes, dependencies and helpers are `def`. +- Use `async def` only for real awaits (ARQ enqueue, async HTTP), and run the synchronous database work in the thread pool. +- Start worker pipelines only through `enqueue_pipeline_for_score_set` (`lib/workflow/kickoff.py`). +- Route tests go in `tests/routers/`. diff --git a/.claude/rules/tests.md b/.claude/rules/tests.md new file mode 100644 index 000000000..6b7142281 --- /dev/null +++ b/.claude/rules/tests.md @@ -0,0 +1,14 @@ +--- +paths: + - "tests/**" +--- + +# Tests + +- Mock only at system boundaries: external services, Redis/ARQ, S3, Slack. Don't mock internal helpers, validators or transforms. +- Assert on outcomes: return values, responses, database state, and calls at system boundaries. Don't assert on internal calls, call counts or log and progress message strings; tests should survive a refactor. +- Unit tests cover edge cases and error paths with external services mocked. Integration tests cover happy paths end to end against the real test database. +- `session.refresh()` before asserting on a modified ORM object. Include negative assertions where they matter (the record that must not exist). +- Name tests `test___` when you write or modify them; don't rename untouched tests. +- Use `@pytest.mark.parametrize` with `ids` instead of near-identical copies. +- Put a fixture in the most specific `conftest.py` that needs it, and lift it to the nearest common one when it's shared. diff --git a/.claude/rules/worker.md b/.claude/rules/worker.md new file mode 100644 index 000000000..3aeebddb8 --- /dev/null +++ b/.claude/rules/worker.md @@ -0,0 +1,23 @@ +--- +paths: + - "src/mavedb/worker/**" + - "tests/worker/**" +--- + +# Worker + +Read `src/mavedb/worker/README.md` first; `best_practices.md`, `job_decorators.md`, `job_managers.md` and `pipeline_management.md` beside it cover job structure, decorators and pipelines. + +## Rules + +- Job signature: `async def job(ctx, job_id, job_manager) -> JobExecutionOutcome`. Register jobs in `worker/jobs/registry.py`. +- Job code never commits. The management decorators and `update_progress()` own commits. +- Start pipelines only through `enqueue_pipeline_for_score_set` (`lib/workflow/kickoff.py`). +- `max_jobs = 2` is deliberate: psycopg2 is synchronous and blocks the worker's event loop. + +## Testing jobs + +- Assert on `JobExecutionOutcome.status` and `.data` in every job test, plus database state for the objects the job changes. For external-service jobs, assert the boundary calls (ClinGen CAR/LDH, UniProt, gnomAD/Athena, ClinVar, S3). +- Let `update_progress()` run unpatched. It commits as a checkpoint in production, and tests should exercise that. +- Use `TransactionSpy` (`tests/helpers/transaction_spy.py`) only in manager and decorator tests, where commit and rollback are the contract. In job tests, check database state instead. +- `mock_database_flush_failure` and `mock_database_rollback_failure` are for infrastructure error-recovery paths that are hard to trigger otherwise. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 000000000..3dac982ca --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,32 @@ +{ + "env": { + "AWS_PROFILE": "mavedb-agent-readonly" + }, + "permissions": { + "deny": ["Bash(* --no-verify*)", "Read(~/.aws/**)", "Read(~/.pgpass)", "Read(~/.ssh/**)"], + "ask": [ + "Edit(**/.claude/settings*.json)", + "Bash(git push *--force*)", + "Bash(git push * -f*)", + "Bash(git push * main*)", + "Bash(git push * mavedb-main*)", + "Bash(git push * mavedb-dev*)", + "Bash(git push * release-*)", + "Bash(git -C * push *--force*)", + "Bash(git -C * push * -f*)", + "Bash(git -C * push * main*)", + "Bash(git -C * push * mavedb-main*)", + "Bash(git -C * push * mavedb-dev*)", + "Bash(git -C * push * release-*)", + "Bash(aws *--profile*)", + "Bash(*AWS_PROFILE=*)", + "Bash(eb *)", + "Bash(*MODE=prodapi*)", + "Bash(poetry add *)", + "Bash(poetry -C * add *)", + "Bash(npm install *)", + "Bash(npm --prefix * install *)", + "Bash(*pip install *)" + ] + } +} diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 000000000..2729397ee --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,5 @@ +# Formatting-only commits, skipped by `git blame` (GitHub reads this file automatically). +# Locally: git config blame.ignoreRevsFile .git-blame-ignore-revs + +# style: apply Ruff 0.17.1 formatting +dcf4e382d2e0bea48bd6c094141727275cbf262f diff --git a/.github/instructions/api.instructions.md b/.github/instructions/api.instructions.md deleted file mode 100644 index a19e68521..000000000 --- a/.github/instructions/api.instructions.md +++ /dev/null @@ -1,165 +0,0 @@ ---- -description: 'MaveDB API patterns — routers, authentication, authorization, endpoints' -applyTo: 'src/mavedb/routers/**/*.py' ---- - -# API Patterns for MaveDB - -## Router Structure - -All routers use: -- `ROUTER_BASE_PREFIX = "/api/v1"` from `src/mavedb/routers/__init__.py` -- `LoggedRoute` as the custom `route_class` for canonical request/response logging -- Kebab-case URL paths: `/score-sets`, `/experiment-sets` - -```python -router = APIRouter( - prefix="/api/v1/score-sets", - tags=["score-sets"], - route_class=LoggedRoute, - responses=shared_responses, -) -``` - -## Authentication - -Three tiers of auth dependency injection: - -| Dependency | Returns | Use When | -|-----------|---------|----------| -| `get_current_user` | `Optional[UserData]` | Public endpoints that behave differently for authenticated users | -| `require_current_user` | `UserData` | Endpoints requiring login | -| `require_current_user_with_email` | `UserData` | Endpoints requiring verified email (write operations) | - -Auth supports two mechanisms: -- **ORCID JWT tokens** — primary auth for web users -- **API keys** — for programmatic access - -```python -@router.get("/{urn}") -def get_score_set( - urn: str, - db: Session = Depends(get_db), - user: Optional[UserData] = Depends(get_current_user), -): - ... -``` - -## Authorization - -Permission checks use the `assert_permission()` helper with an `Action` enum: - -```python -from mavedb.lib.authorization import assert_permission, Action - -assert_permission(user, item, Action.READ) # View -assert_permission(user, item, Action.UPDATE) # Modify -assert_permission(user, item, Action.DELETE) # Delete -assert_permission(user, item, Action.ADD_ROLE) # Manage contributors -``` - -Key authorization behaviors: -- **Private resources return 404** (not 403) to prevent information leakage about existence -- Permission logic dispatches by resource type (ExperimentSet, Experiment, ScoreSet, etc.) -- Admins bypass most permission checks - -## Endpoint Patterns - -### Standard CRUD -```python -@router.get("/", response_model=list[ScoreSetShortModel]) -def list_score_sets(db: Session = Depends(get_db)): ... - -@router.get("/{urn}", response_model=ScoreSetFullModel) -def get_score_set(urn: str, db: Session = Depends(get_db)): ... - -@router.post("/", response_model=ScoreSetSavedModel, status_code=201) -def create_score_set(body: ScoreSetCreateModel, db: Session = Depends(get_db)): ... - -@router.put("/{urn}", response_model=ScoreSetSavedModel) -def update_score_set(urn: str, body: ScoreSetUpdateModel, db: Session = Depends(get_db)): ... - -@router.delete("/{urn}", status_code=204) -def delete_score_set(urn: str, db: Session = Depends(get_db)): ... -``` - -### Background Job Enqueueing -For operations that trigger async processing: -```python -@router.post("/{urn}:publish") -async def publish_score_set( - urn: str, - db: Session = Depends(get_db), - user: UserData = Depends(require_current_user_with_email), - worker: ArqRedis = Depends(get_worker), -): - # ... validation and DB updates ... - await enqueue_pipeline_for_score_set( - db=db, - redis=worker, - pipeline_name="publish_score_set", - score_set=item, - user=user_data.user, - ) -``` - -### Error Responses -Shared error response definitions are used across routers: -```python -responses=shared_responses # Defines 4xx/5xx response schemas -``` - -## Worker Integration - -### Pipeline System - -Most write operations trigger a multi-step pipeline via the worker: - -```python -from mavedb.lib.workflow.kickoff import enqueue_pipeline_for_score_set - -# In a router endpoint: -pipeline, job = await enqueue_pipeline_for_score_set( - db=db, - redis=worker, - pipeline_name="validate_map_annotate_score_set", - score_set=item, - user=user_data.user, - extra_params={"scores_file_key": scores_file_key, ...}, -) -``` - -`enqueue_pipeline_for_score_set` is the single kickoff seam shared by routers and scripts. It fills `score_set_id`, `updater_id` and `correlation_id`, uses `PipelineFactory` to create a `Pipeline` with its `JobRun` and `JobDependency` records, then enqueues the pipeline's `start_pipeline` entrypoint in ARQ. A failed enqueue discards the just-created pipeline records. The worker coordinates the rest — each job runs after its dependencies complete. - -### Job Function Signature - -All job functions follow this signature (the decorator injects `job_manager`): - -```python -@with_pipeline_management -async def create_variants_for_score_set( - ctx: dict, job_id: int, job_manager: JobManager -) -> JobExecutionOutcome: - job = job_manager.get_job() - validate_job_params(["score_set_id", "correlation_id", "updater_id"], job) - # ... business logic using job_manager.db ... - return JobExecutionOutcome.succeeded(data={"variants_created": count}) -``` - -Callers pass only `ctx` and `job_id` when enqueueing. The decorator creates the `JobManager` from the `job_id`. - -### Correlation IDs - -Correlation IDs flow from the API request through the pipeline to each job: - -```python -# In the router — save request context; the kickoff seam reads the correlation ID from it -from mavedb.lib.logging.context import save_to_logging_context - -save_to_logging_context({"score_set_urn": urn}) -await enqueue_pipeline_for_score_set(db=db, redis=worker, pipeline_name="validate_map_annotate_score_set", ...) -``` - -Each job retrieves the correlation ID from its `job_params` and uses `job_manager.save_to_context()` for structured logging. - -For detailed worker conventions, see `.github/instructions/worker.instructions.md` and `src/mavedb/worker/README.md`. diff --git a/.github/instructions/copilot-instructions.md b/.github/instructions/copilot-instructions.md deleted file mode 100644 index 884e83245..000000000 --- a/.github/instructions/copilot-instructions.md +++ /dev/null @@ -1,257 +0,0 @@ ---- -description: 'Primary project guide for MaveDB API — architecture, conventions, domain patterns' -applyTo: '**' ---- - -# MaveDB API — Project Guide - -MaveDB is a bioinformatics database API for Multiplex Assays of Variant Effect (MAVE) datasets. It stores, validates, maps, and publishes functional genomics data using standardized variant representations (HGVS, GA4GH VRS). - -## Architecture - -### Service Architecture -- **FastAPI application** — `src/mavedb/server_main.py`, router-based endpoint organization -- **Background worker** — ARQ/Redis for async processing (variant creation, mapping, publication, annotation) -- **Multi-container setup** — API server, worker, PostgreSQL 15, Redis, moto (local S3), plus external services (cdot-rest, dcd-mapping, seqrepo) -- **Docker config** — `docker-compose-dev.yml` (9 services), multi-stage `Dockerfile` - -### Core Domain Model -- **Hierarchical URN system**: ExperimentSet (`urn:mavedb:00000001`) → Experiment (`00000001-a`) → ScoreSet (`00000001-a-1`) → Variant (ScoreSet URN + `#` + number) -- **Temporary URNs** during development: `tmp:` format, converted to permanent URNs on publication -- **Resource lifecycle**: Draft → Published (with background worker processing for variant creation, mapping, and annotation) -- **URN regex patterns**: `src/mavedb/lib/validation/urn_re.py` -- **URN generation**: `src/mavedb/lib/urns.py` and `temp_urns.py` - -### Key Dependencies (Dependency Injection) -```python -def get_db() -> Generator[Session, Any, None] # Database session -async def get_worker() -> AsyncGenerator[ArqRedis, Any] # Worker queue -def hgvs_data_provider() -> RESTDataProvider # HGVS validation -def get_seqrepo() -> SeqRepo # Sequence retrieval -``` - -## Project Structure - -``` -src/mavedb/ -├── server_main.py # FastAPI app setup, middleware, dependency injection -├── models/ # SQLAlchemy ORM models -├── view_models/ # Pydantic request/response models -├── routers/ # API endpoint handlers -├── worker/ # ARQ background worker system -│ ├── jobs/ # Job function implementations (by category) -│ │ ├── registry.py # Central registry of all jobs, cron definitions -│ │ ├── variant_processing/ # Variant creation, mapping, reverse translation -│ │ ├── external_services/ # ClinGen, ClinVar, gnomAD, UniProt, VEP -│ │ ├── pipeline_management/ # Pipeline entrypoint (start_pipeline) -│ │ └── system/ # Cron maintenance (cleanup stalled jobs) -│ ├── lib/ # Infrastructure layer -│ │ ├── decorators/ # @with_pipeline_management, @with_job_management -│ │ └── managers/ # JobManager, PipelineManager state management -│ └── settings/ # ARQ worker config, lifecycle hooks -├── lib/ # Shared utilities -│ ├── authentication.py # ORCID JWT + API key auth -│ ├── authorization.py # Permission checks -│ ├── exceptions.py # Domain exceptions (MixedTargetError, NonexistentOrcidError, etc.) -│ ├── logging/ # LoggedRoute, logging_context(), save_to_logging_context() -│ ├── urns.py # URN generation -│ └── validation/ # Validators, URN regex, HGVS checks, transform module -├── data_providers/ # External service clients -│ └── services.py # ClinGen, SeqRepo, CDOT integrations -└── scripts/ # Operational Click-based CLI scripts -alembic/ -├── versions/ # Migration files -└── manual_migrations/ # Complex data migration scripts -tests/ -├── conftest.py # Core fixtures (DB, auth, users) -├── helpers/ -│ └── constants.py # Test data constants -└── -``` - -## Naming Conventions - -| Element | Convention | Example | -|---------|-----------|---------| -| Variables & functions | `snake_case` | `score_set_id`, `create_variants_for_score_set` | -| Classes | `PascalCase` | `ScoreSet`, `UserData`, `ProcessingState` | -| Constants | `UPPER_SNAKE_CASE` | `MAPPING_QUEUE_NAME`, `ROUTER_BASE_PREFIX` | -| Enum values | `snake_case` | `ProcessingState.success`, `MappingState.incomplete` | -| Database tables | `snake_case` | `scoresets`, `scoreset_contributors` | -| API endpoints | kebab-case | `/score-sets`, `/experiment-sets` | -| View model aliases | camelCase (auto) | Python `score_set` → JSON `scoreSet` | - -## Living Documentation - -Documentation and instruction files (`.github/instructions/`, `.claude/`, `CLAUDE.md`, `README.md`, etc.) are living documents. When making code changes that render existing instructions or documentation obsolete, update those files as part of the same changeset. Never leave stale docs behind. - -## Commenting Guidelines - -**Core principle: explain WHY, not WHAT. Focus on bioinformatics reasoning.** - -Comment for: -- Complex bioinformatics algorithms (variant mapping, score normalization) -- Business logic rationale (why validation rules exist) -- External API constraints (rate limits, data format requirements) -- Non-obvious thresholds or configuration values - -Do not comment obvious operations, variable assignments, or code that is self-explanatory. - -## Error Handling - -- **Structured logging**: Use `logger` with `extra=logging_context()` for correlation IDs via starlette-context -- **HTTP exceptions**: FastAPI `HTTPException` with appropriate status codes -- **Domain exceptions**: `src/mavedb/lib/exceptions.py` — `MixedTargetError`, `NonexistentOrcidError`, etc. -- **Worker errors**: `send_slack_job_error()` or `send_slack_job_error()` + full logging context -- **Validation errors**: Two distinct classes exist: - - `src/mavedb/lib/validation/exceptions.py` — validation package exceptions - - `src/mavedb/lib/exceptions.py` — legacy `ValidationError` (Django-style, used in some older code) - -## External Integrations - -| Service | Purpose | Client Location | -|---------|---------|----------------| -| HGVS / SeqRepo | Genomic sequence operations, variant validation | `data_providers/services.py` | -| DCD Mapping | Variant mapping and VRS transformation | `data_providers/services.py` | -| CDOT | Transcript/genomic coordinate conversion | REST service in Docker | -| GA4GH VRS | Variant representation standardization | Via DCD Mapping | -| ClinGen Allele Registry | Allele registration and lookup | `data_providers/services.py` | -| ClinGen Linked Data Hub | Functional annotation submission | `data_providers/services.py` | - -## Development Commands - -```bash -# Docker development environment -docker-compose -f docker-compose-dev.yml up --build -d - -# Direct execution (requires env vars) -export PYTHONPATH="${PYTHONPATH}:`pwd`/src" -uvicorn mavedb.server_main:app --reload - -# Run tests -poetry run pytest tests/ - -# Database migrations -alembic upgrade head -alembic revision --autogenerate -m "Description" - -# Operational scripts (Click CLI, dry-run by default) -poetry run python -m mavedb.scripts. -``` - -## Key Reference Files - -- [score_set.py](src/mavedb/models/score_set.py) — Primary data model patterns -- [score_sets.py](src/mavedb/routers/score_sets.py) — Complex router with worker integration -- [registry.py](src/mavedb/worker/jobs/registry.py) — Background job registration -- [score_set.py](src/mavedb/view_models/score_set.py) — Pydantic model hierarchy -- [server_main.py](src/mavedb/server_main.py) — App setup and dependency injection -- [authentication.py](src/mavedb/lib/authentication.py) — Auth patterns -- [conftest.py](tests/conftest.py) — Test fixtures and database setup - -### Naming Conventions -- **Variables & functions**: `snake_case` (e.g., `score_set_id`, `create_variants_for_score_set`) -- **Classes**: `PascalCase` (e.g., `ScoreSet`, `UserData`, `ProcessingState`) -- **Constants**: `UPPER_SNAKE_CASE` (e.g., `MAPPING_QUEUE_NAME`, `DEFAULT_LDH_SUBMISSION_BATCH_SIZE`) -- **Enum values**: `snake_case` (e.g., `ProcessingState.success`, `MappingState.incomplete`) -- **Database tables**: `snake_case` with descriptive association table names (e.g., `scoreset_contributors`, `experiment_set_doi_identifiers`) -- **API endpoints**: kebab-case paths (e.g., `/score-sets`, `/experiment-sets`) - -### Documentation Conventions -*For general Python documentation standards, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific additions:* - -- **Algorithm explanations**: Include comments explaining complex logic, especially URN generation and bioinformatics operations -- **Design decisions**: Comment on why certain architectural choices were made -- **External dependencies**: Explain purpose of external bioinformatics libraries (HGVS, SeqRepo, etc.) -- **Bioinformatics context**: Document biological reasoning behind genomic data processing patterns - -### Commenting Guidelines -**Core Principle: Write self-explanatory code. Comment only to explain WHY, not WHAT.** - -**✅ WRITE Comments For:** -- **Complex bioinformatics algorithms**: Variant mapping algorithms, external service interactions -- **Business logic**: Why specific validation rules exist, regulatory requirements -- **External API constraints**: Rate limits, data format requirements -- **Non-obvious calculations**: Score normalization, statistical methods -- **Configuration values**: Why specific timeouts, batch sizes, or thresholds were chosen - -**❌ AVOID Comments For:** -- **Obvious operations**: Variable assignments, simple loops, basic conditionals -- **Redundant descriptions**: Comments that repeat what the code clearly shows -- **Outdated information**: Comments that don't match current implementation - -### Error Handling Conventions -- **Structured logging**: Always use `logger` with `extra=logging_context()` for correlation IDs -- **HTTP exceptions**: Use FastAPI `HTTPException` with appropriate status codes and descriptive messages -- **Custom exceptions**: Define domain-specific exceptions in `src/mavedb/lib/exceptions.py` -- **Worker job errors**: Send Slack notifications via `send_slack_job_error()` or `send_slack_job_failure()` and log with full context -- **Validation errors**: Use Pydantic validators and raise `ValueError` with clear messages - -### Code Style and Organization Conventions -*For general Python style conventions, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific patterns:* - -- **Async patterns**: Use `async def` for I/O operations, regular functions for CPU-bound work -- **Database operations**: Use SQLAlchemy 2.0 style with `session.scalars(select(...)).one()` -- **Pydantic models**: Separate request/response models with clear inheritance hierarchies -- **Bioinformatics data flow**: Structure code to clearly show genomic data transformations - -### Testing Conventions -*For testing philosophy, mocking boundaries, and conventions see `.github/instructions/testing.instructions.md`. For general Python testing standards, see `.github/instructions/python.instructions.md`. The following are MaveDB-specific patterns:* - -- **Test function naming**: Use descriptive names that reflect bioinformatics operations (e.g., `test_cannot_publish_score_set_without_variants`) -- **Fixtures**: Use `conftest.py` for shared fixtures, especially database and worker setup -- **Mocking**: Mock only at system boundaries (external services, Redis/ARQ, Slack). Do not mock internal helpers or `update_progress` -- **Constants**: Define test data including genomic sequences and variants in `tests/helpers/constants.py` -- **Integration testing**: Test full bioinformatics workflows including external service interactions - -## Codebase Conventions - -### URN Validation -- Use regex patterns from `src/mavedb/lib/validation/urn_re.py` -- Validate URNs in Pydantic models with `@field_validator` -- URN generation logic in `src/mavedb/lib/urns.py` and `temp_urns.py` - -### Worker Jobs (ARQ/Redis) -- **Two-layer architecture**: Infrastructure (decorators + managers) handles lifecycle/state; business layer (jobs/) implements domain logic -- **Job registry**: All jobs registered in `src/mavedb/worker/jobs/registry.py` — `BACKGROUND_FUNCTIONS`, `BACKGROUND_CRONJOBS`, `STANDALONE_JOB_DEFINITIONS` -- **Job function signature**: `async def job_name(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutionOutcome` — `job_manager` is injected by the decorator, not passed by callers -- **Decorators**: `@with_pipeline_management` (most jobs), `@with_job_management` (standalone), `@with_guaranteed_job_run_record` (cron/auto-created JobRun) -- **Pipeline system**: `PipelineFactory.create_pipeline()` creates Pipeline + JobRun + JobDependency records from definitions in `src/mavedb/lib/workflow/definitions.py` -- **Session management**: Task-local DB sessions via `ContextVar` prevent concurrent ARQ jobs from sharing sessions -- **Commit discipline**: Decorators commit lifecycle state changes; `update_progress()` commits as a checkpoint; job code should NOT commit -- **Key job types**: - - `create_variants_for_score_set` - Parse uploaded CSV, create variant records - - `map_variants_for_score_set` - Map variants via DCD Mapping / VRS - - `reverse_translate_variants_for_score_set` - Derive equivalent nucleotide/protein alleles and projection groups via variant-annotation - - `submit_score_set_mappings_to_car/ldh` - Submit to ClinGen services - - `cleanup_stalled_jobs` - Cron job for recovering stuck jobs -- **Enqueueing pipelines**: Routers and scripts call `enqueue_pipeline_for_score_set()` (`src/mavedb/lib/workflow/kickoff.py`), which creates the pipeline via `PipelineFactory.create_pipeline()` and enqueues its `start_pipeline` entrypoint JobRun -- **Detailed documentation**: See `src/mavedb/worker/README.md` and `.github/instructions/worker.instructions.md` - -### View Models (Pydantic) -- **Base model** (`src/mavedb/view_models/base/base.py`) converts empty strings to None and uses camelCase aliases -- **Inheritance patterns**: `Base` → `Create` → `Modify` → `Saved` model hierarchy -- **Field validation**: Use `@field_validator` for single fields, `@model_validator(mode="after")` for cross-field validation -- **URN validation**: Validate URNs with regex patterns from `urn_re.py` in field validators -- **Transform functions**: Use functions in `validation/transform.py` for complex data transformations -- **Separate models**: Request (`Create`, `Modify`) vs response (`Saved`) models with different field requirements - -### External Integrations -- **HGVS/SeqRepo** for genomic sequence operations -- **DCD Mapping** for variant mapping and VRS transformation -- **CDOT** for transcript/genomic coordinate conversion -- **GA4GH VRS** for variant representation standardization -- **ClinGen services** for allele registry and linked data hub submissions - -## Key Files to Reference -- `src/mavedb/models/score_set.py` - Primary data model patterns -- `src/mavedb/routers/score_sets.py` - Complex router with worker integration -- `src/mavedb/worker/jobs/registry.py` - Job registration and available functions -- `src/mavedb/worker/jobs/variant_processing/creation.py` - Reference pipeline job implementation -- `src/mavedb/lib/workflow/definitions.py` - Pipeline and job definitions -- `src/mavedb/view_models/score_set.py` - Pydantic model hierarchy examples -- `src/mavedb/server_main.py` - Application setup and dependency injection -- `src/mavedb/data_providers/services.py` - External service integration patterns -- `src/mavedb/lib/authentication.py` - Authentication and authorization patterns -- `tests/conftest.py` - Test fixtures and database setup -- `docker-compose-dev.yml` - Service architecture and dependencies diff --git a/.github/instructions/database.instructions.md b/.github/instructions/database.instructions.md deleted file mode 100644 index 947fc36cc..000000000 --- a/.github/instructions/database.instructions.md +++ /dev/null @@ -1,133 +0,0 @@ ---- -description: 'MaveDB database patterns — SQLAlchemy models, migrations, query conventions' -applyTo: 'src/mavedb/models/**/*.py,alembic/**/*.py' ---- - -# Database Patterns for MaveDB - -## Model Structure - -### Base Class -All models subclass the `@as_declarative()` `Base` in `src/mavedb/db/base.py`. It declares no columns; it only defaults `__tablename__` to the lowercased class name. Table models declare their own `id` primary key and set `__tablename__` explicitly; view models map `__table__` to a view instead. - -### Common Columns -Most resource models include: -- `creation_date` / `modification_date`: `Date` columns defaulting to today (`modification_date` updates on write) -- `created_by_id` / `modified_by_id`: Foreign keys to the users table (user-owned resources: experiment sets, experiments, score sets, calibrations, calibration controls, collections) -- `urn`: Unique string identifier (temporary `tmp:` or permanent `urn:mavedb:...`) -- `private`: Boolean flag for visibility control -- `approved`: Boolean for moderation status -- `published_date`: Nullable datetime, set on publication -- `extra_metadata`: JSONB column for extensible metadata - -### Relationship Patterns - -**One-to-many** (parent → children): -```python -class Experiment(Base): - experiment_set_id = Column(Integer, ForeignKey("experiment_sets.id"), nullable=False) - experiment_set = relationship("ExperimentSet", back_populates="experiments") -``` - -**Many-to-many with association tables** (simple join): -```python -# Table object for simple joins (no extra columns) -scoreset_contributors = Table( - "scoreset_contributors", - Base.metadata, - Column("scoreset_id", ForeignKey("scoresets.id")), - Column("contributor_id", ForeignKey("contributors.id")), -) -``` - -**Many-to-many with Association Object** (extra columns needed): -```python -# Class-based for additional attributes on the relationship -class ScoreSetPublicationIdentifierAssociation(Base): - scoreset_id = Column(ForeignKey("scoresets.id"), primary_key=True) - publication_identifier_id = Column(ForeignKey("publication_identifiers.id"), primary_key=True) - primary = Column(Boolean, default=False) -``` - -### Valid-Time Versioning (`ValidTime`) -Rows that change over time mix in `ValidTime` (`src/mavedb/db/mixins.py`) instead of being updated or deleted in place: -- `valid_from` / `valid_to`: a row is live while `valid_to` is NULL; history is the half-open window `[valid_from, valid_to)`. -- Reads: filter explicitly with `Model.current` or `Model.live_at(as_of)` (`None` means current; also works on `aliased()` classes). Never assume a query returns only live rows. -- Replacing a row: `supersede_with` (one row) or `supersede_live_where` (bulk) retire the old rows and stamp the new rows' `valid_from` with the same timestamp, so the handoff has no gap. Never retire and then insert separately. -- Withdrawing with no successor: `retire` (one row) or `retire_live_where` (bulk UPDATE). -- `__retire_cascade__`: relationship names whose live `ValidTime` children are retired when the parent is (one level deep). `supersede_live_where` raises on a class that declares it; use `supersede_with` per row. -- Each table adds a partial unique index over its natural key `WHERE valid_to IS NULL`, so a second live row raises `IntegrityError`. -- Content-addressed, immutable rows (e.g. alleles) do not use the mixin. - -### Enum Columns -Use non-native enums stored as strings: -```python -processing_state = Column( - Enum(ProcessingState, native_enum=False, create_constraint=True, length=32), - nullable=True, -) -``` - -### JSONB Columns -Use `JSONB` for flexible structured data: -```python -mapped_scores = Column(JSONB, nullable=True) # Post-mapping variant scores -extra_metadata = Column(JSONB, nullable=True) # User-defined metadata -``` - -### Column Aliasing -When Python attribute names differ from DB column names: -```python -num_variants = Column("variant_count", Integer, nullable=False, default=0) -``` - -## Migrations (Alembic) - -### Standard Migrations -```bash -alembic revision --autogenerate -m "add column to scoresets" -alembic upgrade head -``` - -### Manual Migrations -For complex data transformations that can't be auto-generated: -- Place scripts in `alembic/manual_migrations/` -- Import and call from the version file's `upgrade()` function -- Always include a `downgrade()` path - -### Migration Conventions -- One logical change per migration -- Use descriptive messages: `"add variant_count to scoresets"` not `"update model"` -- Test migrations against a populated database when possible -- For enum changes: use `native_enum=False` to avoid PostgreSQL enum type ALTER issues - -## Query Patterns - -### Fetching -```python -# Single item (raises if not found) -item = db.scalars(select(ScoreSet).where(ScoreSet.urn == urn)).one() - -# Single item (returns None if not found) -item = db.scalars(select(ScoreSet).where(ScoreSet.urn == urn)).one_or_none() - -# List with filters -items = db.scalars( - select(ScoreSet) - .where(ScoreSet.private.is_(False)) - .order_by(ScoreSet.urn) -).all() -``` - -### Visibility Filtering -Private resources return 404 (not 403) to avoid information leakage: -```python -item = db.scalars(select(ScoreSet).where(ScoreSet.urn == urn)).one_or_none() -if item is None: - raise HTTPException(status_code=404) -if item.private and not user_has_access(user, item): - raise HTTPException(status_code=404) # 404, not 403 -``` - -## Materialized Views -Used for pre-computed aggregations. Refresh via worker cron jobs defined in `src/mavedb/worker/jobs/registry.py`. diff --git a/.github/instructions/python.instructions.md b/.github/instructions/python.instructions.md deleted file mode 100644 index 53c526354..000000000 --- a/.github/instructions/python.instructions.md +++ /dev/null @@ -1,112 +0,0 @@ ---- -description: 'MaveDB Python coding conventions — SQLAlchemy, Pydantic, FastAPI patterns' -applyTo: '**/*.py' ---- - -# Python Conventions for MaveDB - -## Style - -- Follow PEP 8 with 4-space indentation -- Use type hints on all function signatures -- Use `from __future__ import annotations` where needed for forward references -- Prefer `list[str]` over `List[str]` (Python 3.10+ style) in new code, but match surrounding code style - -## SQLAlchemy 2.0 Patterns - -Use 2.0-style query syntax throughout: - -```python -# Correct — 2.0 style -item = db.scalars(select(ScoreSet).where(ScoreSet.urn == urn)).one() -items = db.scalars(select(ScoreSet).where(ScoreSet.private.is_(False))).all() - -# Avoid — legacy 1.x style -item = db.query(ScoreSet).filter_by(urn=urn).one() -``` - -Use `session.add()` / `session.flush()` / `session.commit()` appropriately: -- Routers: `db.add()` then `db.commit()` for write operations -- Worker jobs: manage sessions via `setup_db_session()` or `SessionLocal()` - -## Pydantic v2 Patterns - -### Base Model -All view models inherit from a custom `BaseModel` (`src/mavedb/view_models/base/base.py`) that: -- Converts empty strings to `None` via `@model_validator(mode="before")` -- Uses `alias_generator = humps.camelize` for camelCase JSON serialization -- Sets `populate_by_name = True` to accept both Python and camelCase names - -### Model Hierarchy -Follow the established inheritance pattern: -``` -Base (shared fields) -├── Create (POST request body) -├── Update (PUT/PATCH request body) -└── Saved (GET response) - ├── Short (list endpoints, minimal fields) - ├── Full (detail endpoints, all fields) - ├── Admin (admin-only fields) - └── PublicDump (data export) -``` - -### record_type Pattern -Every saved view model must include a `record_type` literal for frontend type discrimination: -```python -class ScoreSetSavedModel(BaseModel): - record_type: Literal["score_set"] = "score_set" -``` - -### Validators -- `@field_validator("field_name", mode="before")` — single field transformations -- `@model_validator(mode="before")` — pre-parse transformations (e.g. ORM → dict conversions) -- `@model_validator(mode="after")` — cross-field validation - -### Synthetic Fields (ORM → View Model) -When SQLAlchemy model attributes don't map 1:1 to view model fields, use `@model_validator(mode="before")`: -```python -@model_validator(mode="before") -@classmethod -def set_computed_field(cls, data): - if hasattr(data, "some_relationship"): - data.__setattr__("computed_count", len(data.some_relationship)) - return data -``` - -### all_fields_optional_model Decorator -Used to create PATCH-style update models from existing models: -```python -@all_fields_optional_model -class ScoreSetUpdateModel(ScoreSetCreateModel): - ... -``` - -## Enum Conventions - -- Enum values use `snake_case`: `ProcessingState.success`, `MappingState.incomplete` -- Database enums use `native_enum=False` with `String` column type -- Define in `src/mavedb/lib/` or alongside the model that uses them - -## Async Patterns - -- Router endpoints: `async def` when they await worker/external calls, regular `def` for synchronous DB operations -- Worker jobs: `async def` (ARQ requirement) -- Database operations are synchronous (SQLAlchemy sync sessions) - -## Logging - -Use structured logging with starlette-context correlation IDs: -```python -import logging -from mavedb.lib.logging.context import logging_context, save_to_logging_context - -logger = logging.getLogger(__name__) -logger.info("Processing score set", extra=logging_context()) -``` - -## Scripts - -Operational scripts use Click with a `@with_database_session` decorator: -- Always default to `dry_run=True` -- Located in `src/mavedb/scripts/` -- Run via `poetry run python -m mavedb.scripts.` diff --git a/.github/instructions/testing.instructions.md b/.github/instructions/testing.instructions.md deleted file mode 100644 index 4fd1b1ce6..000000000 --- a/.github/instructions/testing.instructions.md +++ /dev/null @@ -1,88 +0,0 @@ ---- -description: 'Testing philosophy and conventions for the MaveDB API' -applyTo: 'tests/**/*.py' ---- - -# Testing Conventions - -## Outcome-Based Testing - -Test what code does (return values, DB state, external boundary calls), not how it does it (internal method calls, message strings, call sequences). Tests should survive internal refactoring without changes. - -**Assert on:** -- Return values and response objects -- DB state changes (query for created/updated/deleted records) -- External boundary calls (see Mocking Boundaries below) - -**Do not assert on:** -- Internal function invocations (e.g., that a helper was called with specific args) -- Call counts or call sequences on internal methods -- Log or progress message strings - -## Mocking Boundaries - -Only mock at system boundaries — the edges where your code talks to something external: -- External services (APIs, third-party clients) -- Infrastructure (Redis/ARQ, Slack, email) -- Network I/O (`run_in_executor`, HTTP clients) -- File I/O (S3, local filesystem in tests) - -Do NOT mock internal helpers, validators, or data transforms. Test through them. - -## Unit vs Integration Test Responsibilities - -**Unit tests:** Edge cases, error paths, invalid inputs, boundary conditions. Use mocked external services. - -**Integration tests:** Happy paths, end-to-end workflows, DB state verification. Use real DB with test fixtures. - -## Assertion Best Practices - -- Use `session.refresh()` before asserting on modified ORM objects -- Add custom assertion messages to complex assertions where the failure message wouldn't immediately clarify what went wrong -- Include negative assertions where appropriate (verify unwanted records don't exist) -- Don't add messages to trivially clear assertions like `assert len(variants) == 0` - -## Test Naming - -Use the pattern: `test___` - -Examples: -- `test_submit_to_car_when_disabled_skips_submission` -- `test_create_score_set_returns_422_when_missing_target` - -Apply to tests being modified; don't rename all tests at once. - -## Parametrization - -Use `@pytest.mark.parametrize` with descriptive `ids` when the same logic is tested across multiple states. Prefer parametrization over copy-pasting near-identical tests. - -## Fixtures - -- Keep fixtures minimal and composable -- Define fixtures in the most specific `conftest.py` where they're needed -- Don't duplicate fixtures across test classes — lift shared ones to the nearest common conftest -- Use factory fixtures when tests need variants of the same object - ---- - -# Worker-Specific Conventions - -The following conventions apply specifically to `tests/worker/`. - -## Job Test Assertions - -- Assert on `JobExecutionOutcome.status` and `.data` for every job test -- Assert on DB state changes for the domain objects the job modifies -- For external service jobs: assert boundary calls (ClinGen CAR/LDH, UniProt, gnomAD/Athena, S3, ClinVar) - -## Let `update_progress` Run Unpatched - -`update_progress()` calls `session.commit()` as a checkpoint. This is production behavior and should execute in tests. Letting it run means tests verify that checkpoint commits don't break state or interfere with final outcomes. Don't patch it, don't mock it, don't assert on its messages. - -## TransactionSpy Usage - -**USE in manager/decorator tests** (e.g., `test_job_manager.py`, `test_pipeline_manager.py`): The commit/rollback boundary IS the contract here. If someone removes a commit, data silently won't persist in production. DB state checks alone can't catch this because the test session may auto-commit on teardown. - -**USE `mock_database_flush_failure` / `mock_database_rollback_failure`**: These simulate DB errors that are genuinely hard to reproduce otherwise. Valuable for testing error recovery paths in infrastructure code. - -**DO NOT USE in job-level tests** (e.g., `test_clingen.py`, `test_cleanup.py`, `test_creation.py`): The job's contract is "variants were created" or "stalled jobs were retried," not "session.commit() was called." Use DB state queries instead. diff --git a/.github/instructions/worker.instructions.md b/.github/instructions/worker.instructions.md deleted file mode 100644 index 8d654c2d5..000000000 --- a/.github/instructions/worker.instructions.md +++ /dev/null @@ -1,192 +0,0 @@ ---- -description: 'MaveDB worker patterns — jobs, decorators, managers, pipelines' -applyTo: 'src/mavedb/worker/**/*.py' ---- - -# Worker Conventions for MaveDB - -*For comprehensive documentation with walkthroughs and examples, see `src/mavedb/worker/README.md` and linked docs.* - -## Architecture - -The worker is a two-layer system: - -- **Infrastructure layer** (`lib/decorators/`, `lib/managers/`): Handles job lifecycle, state persistence, error recovery, pipeline coordination. Developers rarely modify this. -- **Business layer** (`jobs/`): Implements domain logic (variant creation, mapping, external service calls). This is where most new code goes. - -Decorators bridge the two layers. Job functions focus purely on business logic and return a `JobExecutionOutcome`. Decorators handle lifecycle state, commits, error recovery, and pipeline coordination automatically. - -## Job Function Contract - -Every job function follows this signature: - -```python -@with_pipeline_management -async def my_job(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutionOutcome: -``` - -- `ctx`: ARQ context dict containing `db`, `redis`, `hdp` (HGVS data provider), `pool`, `state` -- `job_id`: `JobRun.id` from the database (passed by the caller / ARQ) -- `job_manager`: Injected by the decorator — **NOT passed by the caller** -- Return: Always a `JobExecutionOutcome` via its factory methods - -**Callers enqueue jobs with only the job function name and `job_id`.** The decorator injects `job_manager` as a keyword argument before the function executes. - -## Decorator Rules - -| Decorator | Use For | Stacking | -|-----------|---------|----------| -| `@with_pipeline_management` | Jobs that belong (or may belong) to a pipeline | Use alone — it wraps `@with_job_management` internally | -| `@with_job_management` | Standalone jobs without pipeline coordination | Use alone or under `@with_guaranteed_job_run_record` | -| `@with_guaranteed_job_run_record(type)` | Cron/standalone jobs that need a `JobRun` record auto-created | Must be outermost; stack with `@with_job_management` only | - -**Most jobs use `@with_pipeline_management`** because it works for both pipeline and non-pipeline jobs. If the job has no pipeline association, the decorator simply skips coordination. - -`@with_guaranteed_job_run_record` is **NOT compatible** with `@with_pipeline_management`. It is only for standalone/cron jobs. - -### Decorator internals - -- All decorators become no-ops when `MAVEDB_TEST_MODE=1` (checked via `is_test_mode()`). This allows tests to call job functions directly with a pre-built `JobManager`. -- `ensure_session_ctx()` creates a task-local DB session via `ContextVar`, preventing concurrent ARQ jobs from sharing or closing each other's sessions. -- `with_pipeline_management` wraps `with_job_management` internally by calling `with_job_management(func)` inside `_execute_managed_pipeline`. Do not stack them manually. - -## JobManager API (What Job Code Uses) - -From within a job function, use `job_manager` for: - -```python -# Access the job's DB record and parameters -job = job_manager.get_job() # Returns JobRun ORM object -params = job.job_params # Dict of job parameters (JSONB) - -# Access the database session -score_set = job_manager.db.scalars(select(ScoreSet).where(...)).one() - -# Progress tracking (each call commits as a checkpoint by default) -job_manager.update_progress(current, total, message) -job_manager.update_progress(50, 100, "Halfway done", commit=False) # Skip checkpoint - -# Logging context -job_manager.save_to_context({"score_set_id": 123, "correlation_id": "abc"}) -logger.info("Processing", extra=job_manager.logging_context()) -``` - -**Do not call** `start_job()`, `succeed_job()`, `fail_job()`, `error_job()`, or `complete_job()` from job code. The decorator handles these based on the `JobExecutionOutcome` you return. - -## Session & Commit Discipline - -- **Decorators handle commits** for job lifecycle state transitions (start, complete, fail, retry) -- **`update_progress()` commits by default** as a checkpoint — this commits ALL pending session changes, so call it only at safe transaction boundaries. Pass `commit=False` to skip. -- **Job code should NOT call `db.commit()`** — use `db.flush()` if you need generated IDs before the decorator commits -- **PipelineManager commits before its async Redis enqueue loop** to release PostgreSQL row locks and prevent deadlocks (psycopg2 is synchronous, so a blocked UPDATE would freeze asyncio) - -## Return Values (JobExecutionOutcome) - -Always return using factory methods: - -```python -return JobExecutionOutcome.succeeded(data={"variants_created": count}) -return JobExecutionOutcome.failed(reason="No mapping records found", data={...}) -return JobExecutionOutcome.failed(reason="HGVS parse error", failure_category=FailureCategory.DATA_ERROR) -return JobExecutionOutcome.skipped(data={"reason": "Feature disabled"}) -# For unhandled exceptions: let them propagate — the decorator catches and creates .errored() -``` - -The optional `failure_category` parameter on `.failed()` and `.errored()` controls retry eligibility. Categories in `RETRYABLE_FAILURE_CATEGORIES` (e.g., `NETWORK_ERROR`, `TIMEOUT`, `SERVICE_UNAVAILABLE`) enable automatic retries. When a job doesn't set an explicit category, the decorator classifies unhandled exceptions via `classify_exception()` (e.g., `ConnectionError` → `NETWORK_ERROR`). Unclassifiable exceptions default to `UNKNOWN` (not retryable). - -**Do not return `.errored()` from job code.** Let unhandled exceptions propagate; the decorator catches them, classifies the failure, marks the job as ERRORED, sends Slack alerts, and handles retry logic. - -## Parameter Access Pattern - -Job parameters live in `JobRun.job_params` (JSONB column), not in function arguments: - -```python -job = job_manager.get_job() - -_job_required_params = ["score_set_id", "correlation_id", "updater_id"] -validate_job_params(_job_required_params, job) - -score_set_id = job.job_params["score_set_id"] -correlation_id = job.job_params["correlation_id"] -``` - -Always call `validate_job_params()` (from `worker.jobs.utils.setup`) before accessing params. - -Parameters with `None` values in pipeline definitions are filled at runtime from `pipeline_params` passed by the router/script when creating the pipeline. - -## Error Handling - -- **Business failures** (validation errors, missing data): Return `JobExecutionOutcome.failed(reason=..., failure_category=...)` with an explicit `FailureCategory` for retry control. -- **Unhandled exceptions**: Let them propagate. The decorator catches them, classifies the exception via `classify_exception()`, marks the job as ERRORED, sends a Slack alert, and evaluates retry eligibility. -- **External service disabled/unavailable**: Return `JobExecutionOutcome.skipped()` if a config check shows the service is disabled. Let connection errors propagate for retry handling. -- **Retry eligibility**: Determined by `should_retry()` which checks `retry_count < max_retries` and `failure_category in RETRYABLE_FAILURE_CATEGORIES`. -- **Failure classification**: `classify_exception()` in `utils.py` maps infrastructure exceptions to categories (`ConnectionError` → `NETWORK_ERROR`, `TimeoutError` → `TIMEOUT`, `OSError` → `NETWORK_ERROR`). Unmapped exceptions default to `UNKNOWN`. Job-level explicit `failure_category` on the outcome takes priority over automatic classification. -- **Slack safety**: `send_slack_job_failure()` and `send_slack_job_error()` catch their own exceptions internally (logging critical on failure), so Slack outages never interfere with job lifecycle management or error recovery in the decorators. -- **Stale RUNNING recovery**: `start_job()` accepts RUNNING as a startable status (alongside QUEUED and PENDING). When ARQ re-delivers a job after a worker crash, `start_job()` logs a warning and resets the start timestamp rather than raising `JobTransitionError`. -- **Concurrency limit**: `max_jobs = 2` in `ArqWorkerSettings` prevents event loop starvation from sync psycopg2 DB calls. With the default `max_jobs=10`, multiple concurrent jobs issuing blocking DB operations can starve the asyncio event loop. -- **Fast lane**: `start_pipeline` and `create_variants_for_score_set` run on a separate queue (`FAST_LANE_QUEUE_NAME`) consumed by a second worker process (`ArqFastLaneWorkerSettings`), so long mapping jobs can't block new pipelines. Every `enqueue_job` and every `arq.jobs.Job(...)` status/abort call must pass `_queue_name=arq_queue_name(job_run)`. - -## Pipeline Lifecycle (Brief) - -1. Router or script calls `enqueue_pipeline_for_score_set()` (`lib/workflow/kickoff.py`) → `PipelineFactory.create_pipeline()` creates `Pipeline`, `JobRun`, and `JobDependency` records -2. The same call enqueues the `start_pipeline` entrypoint job in ARQ -3. `start_pipeline` runs → its `@with_pipeline_management` decorator starts the pipeline and calls `coordinate_pipeline()` -4. `coordinate_pipeline()` finds PENDING jobs whose dependencies are met → marks them QUEUED → enqueues in ARQ -5. Each job runs → after completion, its decorator calls `coordinate_pipeline()` again -6. Cycle repeats until all jobs complete or the pipeline fails/is cancelled - -Pipeline definitions live in `src/mavedb/lib/workflow/definitions.py`. The `PipelineFactory` (in `src/mavedb/lib/workflow/pipeline_factory.py`) reads these definitions and creates the DB records. - -*For full details, see `src/mavedb/worker/pipeline_management.md`.* - -## Adding a New Pipeline Job - -1. Create the job function in `src/mavedb/worker/jobs//.py` -2. Decorate with `@with_pipeline_management` -3. Follow the signature: `async def job_name(ctx, job_id, job_manager) -> JobExecutionOutcome` -4. Export from the category's `__init__.py` -5. Register in `src/mavedb/worker/jobs/registry.py` → add to `BACKGROUND_FUNCTIONS` -6. Add a `JobDefinition` entry to the relevant pipeline in `src/mavedb/lib/workflow/definitions.py` - -## Adding a Standalone/Cron Job - -1. Create the job function in `src/mavedb/worker/jobs//.py` -2. Stack `@with_guaranteed_job_run_record("job_type")` (outer) + `@with_job_management` (inner) -3. Export from the category's `__init__.py` -4. Register in `src/mavedb/worker/jobs/registry.py` → add to `BACKGROUND_FUNCTIONS` -5. For cron: also add to `BACKGROUND_CRONJOBS` with schedule -6. Optionally add to `STANDALONE_JOB_DEFINITIONS` if the job needs to be invoked via operational scripts - -## Testing - -- Decorators are no-ops in test mode (`MAVEDB_TEST_MODE=1`). Tests call job functions directly, passing a real `JobManager` instance. -- Assert on `JobExecutionOutcome.status` and `.data` for every job test. -- Assert on DB state changes (query for created/updated/deleted records). -- Let `update_progress()` run unpatched — its commit behavior is production behavior that should be tested. -- Mock only at system boundaries (external APIs, S3, Slack). Do not mock internal helpers. -- Use `TransactionSpy` in manager/decorator tests only, not in job-level tests. - -*For full testing conventions, see `.github/instructions/testing.instructions.md`.* - -## Key Reference Files - -| File | Purpose | -|------|---------| -| `jobs/registry.py` | All registered job functions, cron definitions, standalone definitions | -| `jobs/variant_processing/creation.py` | Reference pipeline job implementation | -| `jobs/system/cleanup.py` | Reference standalone cron job implementation | -| `lib/decorators/pipeline_management.py` | Pipeline decorator (coordinates after job completion) | -| `lib/decorators/job_management.py` | Job lifecycle decorator (start/complete/error handling) | -| `lib/decorators/utils.py` | Session management (`ensure_session_ctx`), test mode (`is_test_mode`) | -| `lib/managers/job_manager.py` | Job state management (used by decorators and job code) | -| `lib/managers/pipeline_manager.py` | Pipeline coordination, dependency resolution, job enqueueing | -| `lib/managers/constants.py` | Status groupings (`TERMINAL_JOB_STATUSES`, `STARTABLE_JOB_STATUSES`, etc.) | -| `lib/managers/exceptions.py` | Exception hierarchy (`JobStateError`, `PipelineCoordinationError`, etc.) | -| `settings/worker.py` | `ArqWorkerSettings` (main) and `ArqFastLaneWorkerSettings` classes (ARQ worker configuration) | -| `settings/lifecycle.py` | Worker startup/shutdown hooks, `standalone_ctx()` | -| `src/mavedb/lib/workflow/definitions.py` | Pipeline and job definitions (`PIPELINE_DEFINITIONS`) | -| `src/mavedb/lib/workflow/pipeline_factory.py` | Creates Pipeline + JobRun + JobDependency records | -| `src/mavedb/lib/types/workflow.py` | `JobExecutionOutcome`, `JobDefinition`, `PipelineDefinition` types | -| `src/mavedb/models/job_run.py` | `JobRun` ORM model | -| `src/mavedb/models/pipeline.py` | `Pipeline` ORM model | -| `src/mavedb/models/enums/job_pipeline.py` | `JobStatus`, `PipelineStatus`, `DependencyType`, `FailureCategory` enums | diff --git a/.github/workflows/run-tests-on-push.yml b/.github/workflows/run-tests-on-push.yml index d03a77591..d2d47cd01 100644 --- a/.github/workflows/run-tests-on-push.yml +++ b/.github/workflows/run-tests-on-push.yml @@ -10,9 +10,9 @@ env: LOG_CONFIG: test jobs: - run-mypy-3_11: + run-checks: runs-on: ubuntu-latest - name: MyPy checks on Python 3.11 + name: Lint, format and type checks (bin/check) steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -24,23 +24,7 @@ jobs: - run: pip install --upgrade pip - run: pip install poetry - run: poetry install --with dev --extras server - - run: poetry run mypy src/ - - run-ruff-lint: - runs-on: ubuntu-latest - name: Ruff linting on Python 3.11 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: "3.11" - cache: 'pip' - - run: sudo apt-get update - - run: sudo apt-get install -y tabix - - run: pip install --upgrade pip - - run: pip install poetry - - run: poetry install --with dev --extras server - - run: poetry run ruff check + - run: bin/check run-tests-3_11-core-dependencies: runs-on: ubuntu-latest @@ -56,10 +40,10 @@ jobs: - run: poetry install --with dev - name: Run fast tests on PRs if: github.event_name == 'pull_request' - run: poetry run pytest tests/ -m "not network and not slow" + run: bin/t tests/ -m "not network and not slow" - name: Run full tests on main and release branches if: github.event_name == 'push' - run: poetry run pytest tests/ + run: bin/t tests/ run-tests-3_12-core-dependencies: runs-on: ubuntu-latest @@ -75,10 +59,10 @@ jobs: - run: poetry install --with dev - name: Run fast tests on PRs if: github.event_name == 'pull_request' - run: poetry run pytest tests/ -m "not network and not slow" + run: bin/t tests/ -m "not network and not slow" - name: Run all tests on main and release branches if: github.event_name == 'push' - run: poetry run pytest tests/ + run: bin/t tests/ run-tests-3_11: runs-on: ubuntu-latest @@ -98,12 +82,12 @@ jobs: if: github.event_name == 'pull_request' run: | mkdir -p coverage - poetry run pytest tests/ -m "not network and not slow" --show-capture=stdout --cov=src --cov-report=term-missing --cov-report=lcov:coverage/lcov.info + bin/t tests/ -m "not network and not slow" --show-capture=stdout --cov=src --cov-report=term-missing --cov-report=lcov:coverage/lcov.info - name: Run all tests with coverage on main and release branches if: github.event_name == 'push' run: | mkdir -p coverage - poetry run pytest tests/ --show-capture=stdout --cov=src --cov-report=term-missing --cov-report=lcov:coverage/lcov.info + bin/t tests/ --show-capture=stdout --cov=src --cov-report=term-missing --cov-report=lcov:coverage/lcov.info - name: Upload coverage to Coveralls uses: coverallsapp/github-action@v2 with: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 529ceabb7..cc0085c29 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,9 +1,18 @@ +# Ruff runs from the Poetry environment so poetry.lock is the only place its version is pinned. +# --force-exclude keeps pyproject's `exclude` in effect when pre-commit passes file paths explicitly. repos: - - repo: https://github.com/astral-sh/ruff-pre-commit - # Ruff version. - rev: v0.6.8 + - repo: local hooks: - # Run the linter. - id: ruff - # Run the formatter. + name: ruff check + entry: poetry run ruff check --fix --force-exclude + language: system + types_or: [python, pyi] + require_serial: true - id: ruff-format + name: ruff format + entry: poetry run ruff format --force-exclude + language: system + # Ruff also formats Python code blocks in Markdown; bin/check and CI check them too. + types_or: [python, pyi, markdown] + require_serial: true diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..075c770ab --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,37 @@ +# AGENTS.md — MaveDB API + +Facts about this repo that you can't work out from the code. Keep this file short. Guidance for specific paths goes in `.claude/rules/`, which loads when you touch those paths. Rules shared with the other MaveDB repos are in the workspace `AGENTS.md` one folder up. Update all of them in the same change that makes any of it untrue. + +The MaveDB API is a FastAPI app plus an ARQ/Redis worker over PostgreSQL. The web app and the user docs live in `VariantEffect/mavedb-ui`. + +## Setup + +- `poetry install --with dev --extras server`, on Python 3.11 or 3.12. +- Run everything through `poetry run`. The host Python has none of the dependencies. +- Tests need PostgreSQL server binaries on `PATH`, because pytest-postgresql runs `pg_ctl`, which refuses to run as root. Server-extras tests also need `tabix`. + +## Commands + +| Task | Command | +|---|---| +| Lint, format check, types | `bin/check` | +| Targeted tests | `bin/t tests/routers/test_score_set.py -k publish` | +| What PR CI runs | `bin/t tests/ -m "not network and not slow"` | +| Format | `poetry run ruff format` | +| Migrations | `poetry run alembic upgrade head` | +| Operator scripts | `poetry run python -m mavedb.scripts.` (dry run unless `--commit`) | + +- Run `bin/check` after every edit. It takes seconds. +- Before handing work back, run the targeted tests for what you touched. The full suite is about 4,700 tests and takes roughly 20 minutes, so leave it to CI unless asked. +- If the pre-commit hook rewrites files, re-stage them and commit again. Never use `--no-verify`. +- Change Ruff's version or rule set only in a commit of its own. + +## Rules + +- Private records return 404, never 403, so a response doesn't reveal that they exist. +- A route's permission check covers only the entity it names. Anything a read fans out to needs its own check through a `Viewer` (see `.claude/rules/permissions.md`). +- Never write an `async def` that has no real await (#881). +- Changes reachable from a route or a serving `lib/` module need tests, and so do scripts that emit a recurring public artifact such as the data dump. One-shot operator scripts and manual migrations don't. +- Never add `slow` or `network` markers to get green; those markers drop a test from PR CI. +- Run operator scripts with `--commit` only against a local database unless asked. Never commit to production or staging databases without explicit permission. +- Never write to `mapped_variants` or `variant_translations`; they're legacy and read-only, and new mapping data goes to mapping records and alleles. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/Makefile b/Makefile deleted file mode 100644 index 16f8319bc..000000000 --- a/Makefile +++ /dev/null @@ -1,23 +0,0 @@ -VENV := .venv/bin - -.DEFAULT_GOAL := help - -.PHONY: help -help: - @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf " %-12s %s\n", $$1, $$2}' - -.PHONY: dev -dev: ## Install deps including local editable variant-annotation - poetry install --extras server - -.PHONY: test -test: ## Run the test suite - $(VENV)/pytest tests/ - -.PHONY: lint -lint: ## Check code with ruff - $(VENV)/ruff check src/ tests/ - -.PHONY: format -format: ## Format code with ruff - $(VENV)/ruff format src/ tests/ diff --git a/README.md b/README.md index 8155e470c..770016603 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Or add `mavedb` to your Python project's dependencies. ### Prerequisites -- Python 3.11 or later +- Python 3.11 or 3.12 (the versions CI tests) - PIP - [Poetry](https://python-poetry.org/) for building and publishing distributions. For details on installing poetry, consult its [documentation](https://python-poetry.org/docs/#installation). @@ -46,8 +46,21 @@ poetry publish -r pypi -u -p To build and publish the package in one go, just pass the `--build` flag to the publish command. -For use as a server, this distribution includes an optional set of dependencies, which are only invoked if the package -is installed with `poetry install mavedb --extras server`. +For use as a server, this distribution includes an optional set of dependencies, which are only installed with +`poetry install --extras server`. + +### Development setup + +Once per clone: + +``` +poetry install --with dev --extras server +poetry run pre-commit install +git config blame.ignoreRevsFile .git-blame-ignore-revs +``` + +The pre-commit hooks run Ruff's linter and formatter. The `git config` line makes `git blame` skip formatting-only +commits. Check and test commands are in [AGENTS.md](AGENTS.md). ### Running a local version of the API server diff --git a/alembic/versions/4dbf24ed1857_add_unique_constraint_on_replaces_id.py b/alembic/versions/4dbf24ed1857_add_unique_constraint_on_replaces_id.py index b3f2e5905..2acbbd681 100644 --- a/alembic/versions/4dbf24ed1857_add_unique_constraint_on_replaces_id.py +++ b/alembic/versions/4dbf24ed1857_add_unique_constraint_on_replaces_id.py @@ -5,24 +5,25 @@ Create Date: 2026-05-13 11:22:59.646605 """ + from alembic import op # revision identifiers, used by Alembic. -revision = '4dbf24ed1857' -down_revision = '398067c53257' +revision = "4dbf24ed1857" +down_revision = "398067c53257" branch_labels = None depends_on = None def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_index('ix_scoresets_replaces_id', table_name='scoresets') - op.create_index(op.f('ix_scoresets_replaces_id'), 'scoresets', ['replaces_id'], unique=True) + op.drop_index("ix_scoresets_replaces_id", table_name="scoresets") + op.create_index(op.f("ix_scoresets_replaces_id"), "scoresets", ["replaces_id"], unique=True) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_index(op.f('ix_scoresets_replaces_id'), table_name='scoresets') - op.create_index('ix_scoresets_replaces_id', 'scoresets', ['replaces_id'], unique=False) + op.drop_index(op.f("ix_scoresets_replaces_id"), table_name="scoresets") + op.create_index("ix_scoresets_replaces_id", "scoresets", ["replaces_id"], unique=False) # ### end Alembic commands ### diff --git a/bin/check b/bin/check new file mode 100755 index 000000000..57d4dd638 --- /dev/null +++ b/bin/check @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +# Fast static checks: lint, format and types. Tests run through bin/t. +# Runs every check before exiting so one invocation reports all failures. +set -uo pipefail +cd "$(dirname "$0")/.." + +status=0 +poetry run ruff check || status=1 +poetry run ruff format --check || status=1 +poetry run mypy src/ || status=1 +exit $status diff --git a/bin/t b/bin/t new file mode 100755 index 000000000..4902266f1 --- /dev/null +++ b/bin/t @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +# Targeted tests: bin/t [pytest args] +# Requires a target because the full suite takes ~20 minutes; pass tests/ to run it deliberately. +set -euo pipefail + +if [ $# -eq 0 ]; then + echo "usage: bin/t [pytest args] (pass tests/ for the full suite)" >&2 + exit 2 +fi +exec poetry run pytest "$@" diff --git a/poetry.lock b/poetry.lock index f204b6ff5..1e8ec173f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -4248,30 +4248,30 @@ pyasn1 = ">=0.1.3" [[package]] name = "ruff" -version = "0.6.8" +version = "0.17.1" description = "An extremely fast Python linter and code formatter, written in Rust." optional = false python-versions = ">=3.7" groups = ["dev"] files = [ - {file = "ruff-0.6.8-py3-none-linux_armv6l.whl", hash = "sha256:77944bca110ff0a43b768f05a529fecd0706aac7bcce36d7f1eeb4cbfca5f0f2"}, - {file = "ruff-0.6.8-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:27b87e1801e786cd6ede4ada3faa5e254ce774de835e6723fd94551464c56b8c"}, - {file = "ruff-0.6.8-py3-none-macosx_11_0_arm64.whl", hash = "sha256:cd48f945da2a6334f1793d7f701725a76ba93bf3d73c36f6b21fb04d5338dcf5"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:677e03c00f37c66cea033274295a983c7c546edea5043d0c798833adf4cf4c6f"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:9f1476236b3eacfacfc0f66aa9e6cd39f2a624cb73ea99189556015f27c0bdeb"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:6f5a2f17c7d32991169195d52a04c95b256378bbf0de8cb98478351eb70d526f"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:5fd0d4b7b1457c49e435ee1e437900ced9b35cb8dc5178921dfb7d98d65a08d0"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:f8034b19b993e9601f2ddf2c517451e17a6ab5cdb1c13fdff50c1442a7171d87"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:6cfb227b932ba8ef6e56c9f875d987973cd5e35bc5d05f5abf045af78ad8e098"}, - {file = "ruff-0.6.8-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6ef0411eccfc3909269fed47c61ffebdcb84a04504bafa6b6df9b85c27e813b0"}, - {file = "ruff-0.6.8-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:007dee844738c3d2e6c24ab5bc7d43c99ba3e1943bd2d95d598582e9c1b27750"}, - {file = "ruff-0.6.8-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:ce60058d3cdd8490e5e5471ef086b3f1e90ab872b548814e35930e21d848c9ce"}, - {file = "ruff-0.6.8-py3-none-musllinux_1_2_i686.whl", hash = "sha256:1085c455d1b3fdb8021ad534379c60353b81ba079712bce7a900e834859182fa"}, - {file = "ruff-0.6.8-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:70edf6a93b19481affd287d696d9e311388d808671bc209fb8907b46a8c3af44"}, - {file = "ruff-0.6.8-py3-none-win32.whl", hash = "sha256:792213f7be25316f9b46b854df80a77e0da87ec66691e8f012f887b4a671ab5a"}, - {file = "ruff-0.6.8-py3-none-win_amd64.whl", hash = "sha256:ec0517dc0f37cad14a5319ba7bba6e7e339d03fbf967a6d69b0907d61be7a263"}, - {file = "ruff-0.6.8-py3-none-win_arm64.whl", hash = "sha256:8d3bb2e3fbb9875172119021a13eed38849e762499e3cfde9588e4b4d70968dc"}, - {file = "ruff-0.6.8.tar.gz", hash = "sha256:a5bf44b1aa0adaf6d9d20f86162b34f7c593bfedabc51239953e446aefc8ce18"}, + {file = "ruff-0.17.1-py3-none-linux_armv6l.whl", hash = "sha256:1796c41eafec66f4635125dd1e074a7aa27b8d20c187d484360acf11d247fc4a"}, + {file = "ruff-0.17.1-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:8eadb09a788010be29c895418bdfa513e4e1c65e64b94e5b19e4cd381f6cc7fb"}, + {file = "ruff-0.17.1-py3-none-macosx_11_0_arm64.whl", hash = "sha256:29b454f18ca3eeaf37fbf140841dcf27d36109e29dee9aae8acecb68c26c6f74"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:872e03dd231744d6c5ec5c8af6be52b71323fc6ee096e4c8b34d9f844ee3b978"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:4c0d6cb5f8fbac77eec9af49f4fc0eaf856b05545e372b0694d24b4e76b1c640"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:78beced4a3c94dfedbee4b999b95b6a27b35820fdc0afbacc6d37a8da26c1cd1"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:7feefc63921202894f0e158b0e32f45a5950d70a7fb693ccb9264816c25b1c05"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:684154861a819bf1f433669d2a0ce9c3e54677c65b8ebb866cc2a7ec86345785"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:a6ef98e51de8ef38082f90653ceeb84944bd7673783eea13e1647c5b9b064189"}, + {file = "ruff-0.17.1-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:8f9aa5b5bd5fa1937803ea609c8f153b8de39bf3e0e149897ae34b0c5b52f467"}, + {file = "ruff-0.17.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:80e53b0e5faa3c5667f035daaa032893ae42b7cfe708c886d39ed131e54688ab"}, + {file = "ruff-0.17.1-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:056c5d5fa0ac790051ae165d7035c7bf956d02de2db557f2e9b31d877a505fca"}, + {file = "ruff-0.17.1-py3-none-musllinux_1_2_i686.whl", hash = "sha256:bd8f5a55b10f371642eacd0e8665a42a8b97da15c9a7187c79e697b77b5e0ce4"}, + {file = "ruff-0.17.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:ce05e6f3973611d9550a600e04bb531061e368baba31610eb57c1509a8f21a0a"}, + {file = "ruff-0.17.1-py3-none-win32.whl", hash = "sha256:1de0030a8aa78c65c5fecb84b2e5c390ac684c0452875a02afc7c083d49a496a"}, + {file = "ruff-0.17.1-py3-none-win_amd64.whl", hash = "sha256:5df76f16580fab58d0f6c98a3ac254a4abceb7b2489b8b678335869cff77b3d7"}, + {file = "ruff-0.17.1-py3-none-win_arm64.whl", hash = "sha256:42400b9a6ff8515ff27796f07f92e0195be011c86e81e381606f58ea5502fef4"}, + {file = "ruff-0.17.1.tar.gz", hash = "sha256:5bb796c5112e9fb9527f2ef2faa0a130d6312d19de92bcfaddd70dbb2d41020f"}, ] [[package]] @@ -5297,4 +5297,4 @@ server = ["aiocache", "alembic", "alembic-utils", "arq", "authlib", "bbi-variant [metadata] lock-version = "2.1" python-versions = "^3.11" -content-hash = "3354617968bfec911fcc172ceedfceeecf71b415356f757f2173c3ac45a4ca0d" +content-hash = "db7b3312ba2b0593e0a225ef66a0cdc71bc11ee2b29ebe472ff496921aee61d2" diff --git a/pyproject.toml b/pyproject.toml index d382064db..2f3c03361 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,7 @@ types-python-jose = "~3.3.4" types-PyYAML = "~6.0.12.20240808" redis = "~5.0.2" requests-mock = "~1.11.0" -ruff = "0.6.8" +ruff = "~0.17.1" SQLAlchemy = { extras = ["mypy"], version = "~2.0.0" } @@ -144,3 +144,7 @@ line-length = 120 # Exclude these files from linting/formatting exclude = ["alembic/manual_migrations"] + +# Ruff's default rule set changes between minor releases, so it is written out here; widen it in its own commit. +[tool.ruff.lint] +select = ["E4", "E7", "E9", "F"] diff --git a/src/mavedb/lib/mave/hgvs.py b/src/mavedb/lib/mave/hgvs.py index 8d253f039..6ae06783c 100644 --- a/src/mavedb/lib/mave/hgvs.py +++ b/src/mavedb/lib/mave/hgvs.py @@ -23,7 +23,7 @@ def validate_hgvs_string( value = value.decode() if not isinstance(value, str): raise ValidationError( - "Variant HGVS values input must be strings. " "'{}' has the type '{}'.".format(value, type(value).__name__) + "Variant HGVS values input must be strings. '{}' has the type '{}'.".format(value, type(value).__name__) ) if value.lower() == "_sy": @@ -48,19 +48,16 @@ def validate_hgvs_string( else: if prefix not in "cn": raise ValidationError( - f"'{value}' is not a transcript variant. The accepted " - f"transcript variant prefixes are 'c.', 'n.'." + f"'{value}' is not a transcript variant. The accepted transcript variant prefixes are 'c.', 'n.'." ) elif column in ("splice", HGVS_SPLICE_COLUMN): if prefix not in "cn": raise ValidationError( - f"'{value}' is not a transcript variant. The accepted " f"transcript variant prefixes are 'c.', 'n.'." + f"'{value}' is not a transcript variant. The accepted transcript variant prefixes are 'c.', 'n.'." ) elif column in ("p", HGVS_PRO_COLUMN): if prefix not in "p": - raise ValidationError( - f"'{value}' is not a protein variant. The accepted " f"protein variant prefix is 'p.'." - ) + raise ValidationError(f"'{value}' is not a protein variant. The accepted protein variant prefix is 'p.'.") else: raise ValueError("Unknown column '{}'. Expected nt, splice or p".format(column)) diff --git a/src/mavedb/lib/permissions/collection.py b/src/mavedb/lib/permissions/collection.py index a629a45e1..f732470a8 100644 --- a/src/mavedb/lib/permissions/collection.py +++ b/src/mavedb/lib/permissions/collection.py @@ -73,8 +73,7 @@ def has_permission(user_data: Optional[UserData], entity: Collection, action: Ac if action not in handlers: supported_actions = ", ".join(a.value for a in handlers.keys()) raise NotImplementedError( - f"Action '{action.value}' is not supported for collection entities. " - f"Supported actions: {supported_actions}" + f"Action '{action.value}' is not supported for collection entities. Supported actions: {supported_actions}" ) return handlers[action]( diff --git a/src/mavedb/lib/permissions/experiment.py b/src/mavedb/lib/permissions/experiment.py index 91f8f6176..2c7e6bdbb 100644 --- a/src/mavedb/lib/permissions/experiment.py +++ b/src/mavedb/lib/permissions/experiment.py @@ -58,8 +58,7 @@ def has_permission(user_data: Optional[UserData], entity: Experiment, action: Ac if action not in handlers: supported_actions = ", ".join(a.value for a in handlers.keys()) raise NotImplementedError( - f"Action '{action.value}' is not supported for experiment entities. " - f"Supported actions: {supported_actions}" + f"Action '{action.value}' is not supported for experiment entities. Supported actions: {supported_actions}" ) return handlers[action]( diff --git a/src/mavedb/lib/permissions/score_set.py b/src/mavedb/lib/permissions/score_set.py index cb61a6ad6..e54b6b87f 100644 --- a/src/mavedb/lib/permissions/score_set.py +++ b/src/mavedb/lib/permissions/score_set.py @@ -60,8 +60,7 @@ def has_permission(user_data: Optional[UserData], entity: ScoreSet, action: Acti if action not in handlers: supported_actions = ", ".join(a.value for a in handlers.keys()) raise NotImplementedError( - f"Action '{action.value}' is not supported for score set entities. " - f"Supported actions: {supported_actions}" + f"Action '{action.value}' is not supported for score set entities. Supported actions: {supported_actions}" ) return handlers[action]( diff --git a/src/mavedb/lib/validation/identifier.py b/src/mavedb/lib/validation/identifier.py index 077b13fc0..7f0d8eb45 100644 --- a/src/mavedb/lib/validation/identifier.py +++ b/src/mavedb/lib/validation/identifier.py @@ -47,7 +47,7 @@ def validate_sra_identifier(identifier: str): or idutils.is_arrayexpress_array(identifier) or idutils.is_arrayexpress_experiment(identifier) ): - raise ValidationError(f"'{identifier} is not a valid SRA, GEO, ArrayExpress or BioProject " "accession.") + raise ValidationError(f"'{identifier} is not a valid SRA, GEO, ArrayExpress or BioProject accession.") def validate_ensembl_identifier(identifier: str): diff --git a/src/mavedb/lib/validation/utilities.py b/src/mavedb/lib/validation/utilities.py index 2712c886a..8c61215e6 100644 --- a/src/mavedb/lib/validation/utilities.py +++ b/src/mavedb/lib/validation/utilities.py @@ -89,13 +89,13 @@ def construct_hgvs_pro(wt: str, mutant: str, position: int, target_seq: Optional # check that the provided 3 letter amino acid codes are valid if wt not in aa_dict_key_1.values(): raise ValueError( - "wt 3 letter amino acid code {} is invalid, " "must be one of the following: {}".format( + "wt 3 letter amino acid code {} is invalid, must be one of the following: {}".format( wt, list(aa_dict_key_1.values()) ) ) if mutant not in aa_dict_key_1.values(): raise ValueError( - "wt 3 letter amino acid code {} is invalid, " "must be one of the following: {}".format( + "wt 3 letter amino acid code {} is invalid, must be one of the following: {}".format( mutant, list(aa_dict_key_1.values()) ) ) diff --git a/src/mavedb/lib/validation/variant.py b/src/mavedb/lib/validation/variant.py index 2d0f5bcf5..f8653b887 100644 --- a/src/mavedb/lib/validation/variant.py +++ b/src/mavedb/lib/validation/variant.py @@ -62,7 +62,7 @@ def validate_hgvs_string( value = value.decode() if not isinstance(value, str): raise ValidationError( - "Variant HGVS values input must be strings. " "'{}' has the type '{}'.".format(value, type(value).__name__) + "Variant HGVS values input must be strings. '{}' has the type '{}'.".format(value, type(value).__name__) ) if value.lower() == "_sy": @@ -87,19 +87,16 @@ def validate_hgvs_string( else: if prefix not in "cn": raise ValidationError( - f"'{value}' is not a transcript variant. The accepted " - f"transcript variant prefixes are 'c.', 'n.'." + f"'{value}' is not a transcript variant. The accepted transcript variant prefixes are 'c.', 'n.'." ) elif column in ("splice", hgvs_splice_column): if prefix not in "cn": raise ValidationError( - f"'{value}' is not a transcript variant. The accepted " f"transcript variant prefixes are 'c.', 'n.'." + f"'{value}' is not a transcript variant. The accepted transcript variant prefixes are 'c.', 'n.'." ) elif column in ("p", hgvs_pro_column): if prefix not in "p": - raise ValidationError( - f"'{value}' is not a protein variant. The accepted " f"protein variant prefix is 'p.'." - ) + raise ValidationError(f"'{value}' is not a protein variant. The accepted protein variant prefix is 'p.'.") else: raise ValueError("Unknown column '{}'. Expected nt, splice or p".format(column)) diff --git a/src/mavedb/scripts/job_runs.py b/src/mavedb/scripts/job_runs.py index 1ff0fdce7..3d71e4285 100644 --- a/src/mavedb/scripts/job_runs.py +++ b/src/mavedb/scripts/job_runs.py @@ -96,7 +96,7 @@ def list_job_runs( click.echo("No job runs match the given filters.") return - click.echo(f"{'ID':>6} {'STATUS':<10} {'TYPE':<24} {'FUNCTION':<36} " f"{'RETRIES':<8} {'CREATED':<26} URN") + click.echo(f"{'ID':>6} {'STATUS':<10} {'TYPE':<24} {'FUNCTION':<36} {'RETRIES':<8} {'CREATED':<26} URN") for j in job_runs: click.echo( f"{j.id:>6} {str(j.status):<10} {j.job_type[:24]:<24} " diff --git a/src/mavedb/scripts/pipelines.py b/src/mavedb/scripts/pipelines.py index 8e7795bb9..c5033c860 100644 --- a/src/mavedb/scripts/pipelines.py +++ b/src/mavedb/scripts/pipelines.py @@ -92,9 +92,7 @@ def list_pipelines( click.echo(f"{'ID':>6} {'STATUS':<12} {'NAME':<32} {'CREATED':<26} URN") for p in pipelines: - click.echo( - f"{p.id:>6} {str(p.status):<12} {p.name[:32]:<32} " f"{_format_dt(p.created_at):<26} {p.urn or '-'}" - ) + click.echo(f"{p.id:>6} {str(p.status):<12} {p.name[:32]:<32} {_format_dt(p.created_at):<26} {p.urn or '-'}") @script_environment.command(name="show-pipeline") diff --git a/src/mavedb/worker/best_practices.md b/src/mavedb/worker/best_practices.md index 92b672617..13c521e6c 100644 --- a/src/mavedb/worker/best_practices.md +++ b/src/mavedb/worker/best_practices.md @@ -17,21 +17,21 @@ async def my_job(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutio score_set_id = job.job_params["score_set_id"] correlation_id = job.job_params["correlation_id"] - job_manager.save_to_context({ - "application": "mavedb-worker", - "function": "my_job", - "resource": score_set_id, - "correlation_id": correlation_id, - }) + job_manager.save_to_context( + { + "application": "mavedb-worker", + "function": "my_job", + "resource": score_set_id, + "correlation_id": correlation_id, + } + ) # 3. Initialize progress job_manager.update_progress(0, 100, "Starting my job.") logger.info("Starting my job", extra=job_manager.logging_context()) # 4. Load domain objects and do work - score_set = job_manager.db.scalars( - select(ScoreSet).where(ScoreSet.id == score_set_id) - ).one() + score_set = job_manager.db.scalars(select(ScoreSet).where(ScoreSet.id == score_set_id)).one() # ... business logic ... @@ -68,8 +68,7 @@ return JobExecutionOutcome.succeeded(data={"variants_created": count}) # Missing data, validation failure, precondition not met if not mapping_records: return JobExecutionOutcome.failed( - reason="No mapping records found for score set", - data={"score_set_id": score_set_id} + reason="No mapping records found for score set", data={"score_set_id": score_set_id} ) ``` @@ -133,12 +132,14 @@ Until then, the interim pattern (see `map_variants_for_score_set`) is a **livene Always set up logging context early in the job function: ```python -job_manager.save_to_context({ - "application": "mavedb-worker", - "function": "my_job_name", - "resource": score_set.urn, - "correlation_id": correlation_id, -}) +job_manager.save_to_context( + { + "application": "mavedb-worker", + "function": "my_job_name", + "resource": score_set.urn, + "correlation_id": correlation_id, + } +) ``` Then use `job_manager.logging_context()` with every log call: @@ -165,9 +166,7 @@ async def submit_to_external_service(ctx: dict, job_id: int, job_manager: JobMan return JobExecutionOutcome.skipped(data={"reason": "Service submissions disabled"}) # 2. Load required data - score_set = job_manager.db.scalars( - select(ScoreSet).where(ScoreSet.id == job.job_params["score_set_id"]) - ).one() + score_set = job_manager.db.scalars(select(ScoreSet).where(ScoreSet.id == job.job_params["score_set_id"])).one() # 3. Check preconditions has_mappings = job_manager.db.scalar( @@ -349,7 +348,7 @@ assert result.status == "succeeded" ### Use fixtures for job setup The test `conftest.py` provides fixtures for creating `JobRun` records with the right params structure. Use these rather than constructing records manually. -For complete testing guidelines, see `.github/instructions/testing.instructions.md`. +For the rest of the testing conventions, see `.claude/rules/tests.md` and `.claude/rules/worker.md` at the repository root. ## See Also diff --git a/src/mavedb/worker/job_decorators.md b/src/mavedb/worker/job_decorators.md index eafe3fc02..a150d5437 100644 --- a/src/mavedb/worker/job_decorators.md +++ b/src/mavedb/worker/job_decorators.md @@ -11,6 +11,7 @@ Decorators are the bridge between the infrastructure layer and business layer. T ```python from mavedb.worker.lib.decorators.pipeline_management import with_pipeline_management + @with_pipeline_management async def my_job(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutionOutcome: # Business logic here @@ -35,6 +36,7 @@ async def my_job(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutio ```python from mavedb.worker.lib.decorators.job_management import with_job_management + @with_job_management async def my_standalone_job(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutionOutcome: # Business logic here @@ -65,6 +67,7 @@ async def my_standalone_job(ctx: dict, job_id: int, job_manager: JobManager) -> from mavedb.worker.lib.decorators.job_guarantee import with_guaranteed_job_run_record from mavedb.worker.lib.decorators.job_management import with_job_management + @with_guaranteed_job_run_record("cron_job") @with_job_management async def cleanup_stalled_jobs(ctx: dict, job_id: int, job_manager: JobManager) -> JobExecutionOutcome: diff --git a/src/mavedb/worker/job_managers.md b/src/mavedb/worker/job_managers.md index 60f2f3287..6ab9f2ff2 100644 --- a/src/mavedb/worker/job_managers.md +++ b/src/mavedb/worker/job_managers.md @@ -34,11 +34,13 @@ job_manager.update_progress(75, 100, "Annotating", commit=False) # Skip checkpo job_manager.update_status_message("Connecting to ClinGen API...") # Add context for structured logging -job_manager.save_to_context({ - "score_set_id": score_set.id, - "correlation_id": correlation_id, - "function": "create_variants_for_score_set", -}) +job_manager.save_to_context( + { + "score_set_id": score_set.id, + "correlation_id": correlation_id, + "function": "create_variants_for_score_set", + } +) logger.info("Started processing", extra=job_manager.logging_context()) ``` diff --git a/src/mavedb/worker/job_registry.md b/src/mavedb/worker/job_registry.md index 3b7deb5d9..87a82b3d2 100644 --- a/src/mavedb/worker/job_registry.md +++ b/src/mavedb/worker/job_registry.md @@ -37,7 +37,8 @@ BACKGROUND_CRONJOBS: List[CronJob] = [ cron( refresh_materialized_views, name="refresh_all_materialized_views", - hour=20, minute=0, + hour=20, + minute=0, keep_result=timedelta(minutes=2).total_seconds(), ), cron( @@ -103,17 +104,17 @@ async def submit_to_new_service(ctx: dict, job_id: int, job_manager: JobManager) _job_required_params = ["score_set_id", "correlation_id"] validate_job_params(_job_required_params, job) - score_set = job_manager.db.scalars( - select(ScoreSet).where(ScoreSet.id == job.job_params["score_set_id"]) - ).one() + score_set = job_manager.db.scalars(select(ScoreSet).where(ScoreSet.id == job.job_params["score_set_id"])).one() correlation_id = job.job_params["correlation_id"] - job_manager.save_to_context({ - "application": "mavedb-worker", - "function": "submit_to_new_service", - "resource": score_set.urn, - "correlation_id": correlation_id, - }) + job_manager.save_to_context( + { + "application": "mavedb-worker", + "function": "submit_to_new_service", + "resource": score_set.urn, + "correlation_id": correlation_id, + } + ) job_manager.update_progress(0, 100, "Starting NewService submission.") logger.info("Started NewService submission", extra=job_manager.logging_context()) @@ -148,16 +149,18 @@ BACKGROUND_FUNCTIONS: List[Callable] = [ In `src/mavedb/lib/workflow/definitions.py`, add a `JobDefinition` to the appropriate pipeline: ```python -{ - "key": "submit_to_new_service", - "function": "submit_to_new_service", - "type": JobType.MAPPED_VARIANT_ANNOTATION, - "params": { - "correlation_id": None, - "score_set_id": None, +( + { + "key": "submit_to_new_service", + "function": "submit_to_new_service", + "type": JobType.MAPPED_VARIANT_ANNOTATION, + "params": { + "correlation_id": None, + "score_set_id": None, + }, + "dependencies": [("map_variants_for_score_set", DependencyType.SUCCESS_REQUIRED)], }, - "dependencies": [("map_variants_for_score_set", DependencyType.SUCCESS_REQUIRED)], -}, +) ``` ### 5. Write tests @@ -203,7 +206,8 @@ BACKGROUND_CRONJOBS: List[CronJob] = [ cron( my_maintenance_job, name="my_maintenance_job_cron", - hour=4, minute=0, # Run daily at 4:00 AM + hour=4, + minute=0, # Run daily at 4:00 AM keep_result=timedelta(minutes=5).total_seconds(), ), ] @@ -232,17 +236,17 @@ The `ArqWorkerSettings` class (in `settings/worker.py`) brings everything togeth ```python class ArqWorkerSettings: - on_startup = startup # Create ProcessPoolExecutor + on_startup = startup # Create ProcessPoolExecutor on_shutdown = shutdown - on_job_start = on_job_start # Initialize hdp (HGVS data provider), state dict + on_job_start = on_job_start # Initialize hdp (HGVS data provider), state dict on_job_end = on_job_end - after_job_end = log_job # Canonical job logging + after_job_end = log_job # Canonical job logging redis_settings = RedisWorkerSettings functions = BACKGROUND_FUNCTIONS cron_jobs = BACKGROUND_CRONJOBS - max_jobs = MAX_JOBS # 2; limits event-loop starvation from sync DB calls + max_jobs = MAX_JOBS # 2; limits event-loop starvation from sync DB calls job_timeout = JOB_TIMEOUT_SECONDS # 24 hours - allow_abort_jobs = True # Lets cleanup_stalled_jobs abort a wedged RUNNING job + allow_abort_jobs = True # Lets cleanup_stalled_jobs abort a wedged RUNNING job ``` ### Fast-lane worker diff --git a/src/mavedb/worker/lib/managers/utils.py b/src/mavedb/worker/lib/managers/utils.py index 49135599c..a16737977 100644 --- a/src/mavedb/worker/lib/managers/utils.py +++ b/src/mavedb/worker/lib/managers/utils.py @@ -160,8 +160,7 @@ def job_should_be_skipped_due_to_unfulfillable_dependency( if dependency_type == DependencyType.SUCCESS_REQUIRED: if dependent_job_status in (JobStatus.FAILED, JobStatus.ERRORED, JobStatus.SKIPPED, JobStatus.CANCELLED): logger.debug( - f"Job should be skipped due to unfulfillable 'success_required' dependency " - f"({dependent_job_status})." + f"Job should be skipped due to unfulfillable 'success_required' dependency ({dependent_job_status})." ) return True, f"Dependency did not succeed ({dependent_job_status})" @@ -169,8 +168,7 @@ def job_should_be_skipped_due_to_unfulfillable_dependency( if dependency_type == DependencyType.COMPLETION_REQUIRED: if dependent_job_status in (JobStatus.CANCELLED, JobStatus.SKIPPED): logger.debug( - f"Job should be skipped due to unfulfillable 'completion_required' dependency " - f"({dependent_job_status})." + f"Job should be skipped due to unfulfillable 'completion_required' dependency ({dependent_job_status})." ) return True, f"Dependency was not completed successfully ({dependent_job_status})" diff --git a/src/mavedb/worker/pipeline_management.md b/src/mavedb/worker/pipeline_management.md index 9064fdb74..7b68e8f2c 100644 --- a/src/mavedb/worker/pipeline_management.md +++ b/src/mavedb/worker/pipeline_management.md @@ -43,14 +43,14 @@ PIPELINE_DEFINITIONS: dict[str, PipelineDefinition] = { "description": "Human-readable description of what this pipeline does", "job_definitions": [ { - "key": "first_job", # Unique key within pipeline - "function": "first_job_function_name", # Must match registered function name - "type": JobType.VARIANT_CREATION, # Job category enum + "key": "first_job", # Unique key within pipeline + "function": "first_job_function_name", # Must match registered function name + "type": JobType.VARIANT_CREATION, # Job category enum "params": { - "score_set_id": None, # None = filled at runtime from pipeline_params + "score_set_id": None, # None = filled at runtime from pipeline_params "correlation_id": None, }, - "dependencies": [], # No dependencies = runs first + "dependencies": [], # No dependencies = runs first }, { "key": "second_job", diff --git a/tests/helpers/mocks/README.md b/tests/helpers/mocks/README.md index 68a0d5d87..c6849e238 100644 --- a/tests/helpers/mocks/README.md +++ b/tests/helpers/mocks/README.md @@ -38,7 +38,7 @@ score_set = create_mock_score_set(created_by=user) from tests.helpers.mocks.factories import ( create_mock_functional_classification, create_mock_score_calibration, - create_mock_mapped_variant_with_functional_calibration_score_set + create_mock_mapped_variant_with_functional_calibration_score_set, ) classification = create_mock_functional_classification() @@ -48,17 +48,12 @@ mapped_variant = create_mock_mapped_variant_with_functional_calibration_score_se ### For Custom Mock Behavior ```python -from tests.helpers.mocks.mock_utilities import ( - MockObjectWithPydanticFunctionality, - MockVariantCollection -) +from tests.helpers.mocks.mock_utilities import MockObjectWithPydanticFunctionality, MockVariantCollection # Mock that works with both Pydantic validation and attribute access -mock_data = MockObjectWithPydanticFunctionality({ - "camelCaseKey": "value" -}) +mock_data = MockObjectWithPydanticFunctionality({"camelCaseKey": "value"}) assert mock_data["camelCaseKey"] == "value" # Dict access -assert mock_data.camel_case_key == "value" # Attribute access (snake_case) +assert mock_data.camel_case_key == "value" # Attribute access (snake_case) # Mock collection for controlling variant membership tests variants = MockVariantCollection(contains_result=True) diff --git a/tests/helpers/util/score_set.py b/tests/helpers/util/score_set.py index 7a0f3df46..855132d96 100644 --- a/tests/helpers/util/score_set.py +++ b/tests/helpers/util/score_set.py @@ -165,9 +165,9 @@ def create_seq_score_set_with_variants( count_columns_metadata_json_path, ) - assert ( - score_set["numVariants"] == 3 - ), f"Could not create sequence based score set with variants within experiment {experiment_urn}" + assert score_set["numVariants"] == 3, ( + f"Could not create sequence based score set with variants within experiment {experiment_urn}" + ) jsonschema.validate(instance=score_set, schema=ScoreSet.model_json_schema()) return score_set @@ -196,9 +196,9 @@ def create_acc_score_set_with_variants( count_columns_metadata_json_path, ) - assert ( - score_set["numVariants"] == 3 - ), f"Could not create sequence based score set with variants within experiment {experiment_urn}" + assert score_set["numVariants"] == 3, ( + f"Could not create sequence based score set with variants within experiment {experiment_urn}" + ) jsonschema.validate(instance=score_set, schema=ScoreSet.model_json_schema()) return score_set diff --git a/tests/lib/annotation/test_annotate.py b/tests/lib/annotation/test_annotate.py index c7e2215ad..363605b5d 100644 --- a/tests/lib/annotation/test_annotate.py +++ b/tests/lib/annotation/test_annotate.py @@ -342,9 +342,9 @@ def test_pathogenicity_evidence_line_has_evidence_items_are_statement_instances( assert evidence_line.hasEvidenceItems is not None for evidence_item in evidence_line.hasEvidenceItems: # Must be a model instance, not a raw dict - assert not isinstance( - evidence_item, dict - ), "hasEvidenceItems contained a raw dict instead of a model instance" + assert not isinstance(evidence_item, dict), ( + "hasEvidenceItems contained a raw dict instead of a model instance" + ) assert evidence_item.type == "Statement" def test_no_statement_is_built_from_a_private_calibration( diff --git a/tests/lib/csv/test_variant.py b/tests/lib/csv/test_variant.py index 0b67aec3d..62ca3196d 100644 --- a/tests/lib/csv/test_variant.py +++ b/tests/lib/csv/test_variant.py @@ -847,9 +847,9 @@ def test_namespace_discovery_does_not_scan_score_set_variants(self, session, set for statement in statements if "score_calibrations" in statement and " variants" in statement.replace("\n", " ") ] - assert ( - calibration_scans == [] - ), "calibration discovery joined the variants table; it should filter on score_set_id" + assert calibration_scans == [], ( + "calibration discovery joined the variants table; it should filter on score_set_id" + ) def test_base_namespaces_are_all_present_by_default(self, session, setup_lib_db_with_mapped_variant): variant = setup_lib_db_with_mapped_variant.variant diff --git a/tests/lib/permissions/test_collection.py b/tests/lib/permissions/test_collection.py index b9852b611..6e0b3f308 100644 --- a/tests/lib/permissions/test_collection.py +++ b/tests/lib/permissions/test_collection.py @@ -56,9 +56,9 @@ def test_collection_handles_all_actions() -> None: supported = set(COLLECTION_SUPPORTED_ACTIONS) unsupported = set(COLLECTION_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for collections." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for collections." + ) class TestCollectionHasPermission: @@ -168,7 +168,9 @@ class TestCollectionReadActionHandler: PermissionTest("Collection", "published", "anonymous", Action.READ, True), PermissionTest("Collection", "private", "anonymous", Action.READ, False, 404), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_read_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_read_action helper function directly.""" @@ -233,7 +235,9 @@ class TestCollectionUpdateActionHandler: PermissionTest("Collection", "private", "anonymous", Action.UPDATE, False, 404), PermissionTest("Collection", "published", "anonymous", Action.UPDATE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_update_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_update_action helper function directly.""" @@ -300,7 +304,9 @@ class TestCollectionDeleteActionHandler: PermissionTest("Collection", "private", "anonymous", Action.DELETE, False, 404), PermissionTest("Collection", "published", "anonymous", Action.DELETE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_delete_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_delete_action helper function directly.""" @@ -377,7 +383,9 @@ class TestCollectionPublishActionHandler: PermissionTest("Collection", "private", "anonymous", Action.PUBLISH, False, 404), PermissionTest("Collection", "published", "anonymous", Action.PUBLISH, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_publish_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_publish_action helper function directly.""" @@ -462,7 +470,9 @@ class TestCollectionAddExperimentActionHandler: PermissionTest("Collection", "private", "anonymous", Action.ADD_EXPERIMENT, False, 404), PermissionTest("Collection", "published", "anonymous", Action.ADD_EXPERIMENT, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_experiment_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_experiment_action helper function directly.""" @@ -542,7 +552,9 @@ class TestCollectionAddScoreSetActionHandler: PermissionTest("Collection", "private", "anonymous", Action.ADD_SCORE_SET, False, 404), PermissionTest("Collection", "published", "anonymous", Action.ADD_SCORE_SET, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_score_set_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_score_set_action helper function directly.""" @@ -617,7 +629,9 @@ class TestCollectionAddRoleActionHandler: PermissionTest("Collection", "private", "anonymous", Action.ADD_ROLE, False, 404), PermissionTest("Collection", "published", "anonymous", Action.ADD_ROLE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_role_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_role_action helper function directly.""" @@ -710,7 +724,9 @@ class TestCollectionAddBadgeActionHandler: PermissionTest("Collection", "private", "anonymous", Action.ADD_BADGE, False, 404), PermissionTest("Collection", "published", "anonymous", Action.ADD_BADGE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.collection_role if tc.collection_role else 'no_role'}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_badge_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_badge_action helper function directly.""" diff --git a/tests/lib/permissions/test_experiment.py b/tests/lib/permissions/test_experiment.py index 1ea5a6399..d1d11b2b7 100644 --- a/tests/lib/permissions/test_experiment.py +++ b/tests/lib/permissions/test_experiment.py @@ -45,9 +45,9 @@ def test_experiment_handles_all_actions() -> None: supported = set(EXPERIMENT_SUPPORTED_ACTIONS) unsupported = set(EXPERIMENT_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for experiments." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for experiments." + ) class TestExperimentHasPermission: @@ -122,7 +122,9 @@ class TestExperimentReadActionHandler: PermissionTest("Experiment", "published", "anonymous", Action.READ, True), PermissionTest("Experiment", "private", "anonymous", Action.READ, False, 404), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_read_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_read_action helper function directly.""" @@ -169,7 +171,9 @@ class TestExperimentUpdateActionHandler: PermissionTest("Experiment", "private", "anonymous", Action.UPDATE, False, 404), PermissionTest("Experiment", "published", "anonymous", Action.UPDATE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_update_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_update_action helper function directly.""" @@ -214,7 +218,9 @@ class TestExperimentDeleteActionHandler: PermissionTest("Experiment", "private", "mapper", Action.DELETE, False, 404), PermissionTest("Experiment", "published", "mapper", Action.DELETE, False, 403), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_delete_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_delete_action helper function directly.""" @@ -259,7 +265,9 @@ class TestExperimentAddScoreSetActionHandler: PermissionTest("Experiment", "private", "anonymous", Action.ADD_SCORE_SET, False, 404), PermissionTest("Experiment", "published", "anonymous", Action.ADD_SCORE_SET, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_score_set_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_score_set_action helper function directly.""" diff --git a/tests/lib/permissions/test_experiment_set.py b/tests/lib/permissions/test_experiment_set.py index 9e7c42b20..b47d08e16 100644 --- a/tests/lib/permissions/test_experiment_set.py +++ b/tests/lib/permissions/test_experiment_set.py @@ -45,9 +45,9 @@ def test_experiment_set_handles_all_actions() -> None: supported = set(EXPERIMENT_SET_SUPPORTED_ACTIONS) unsupported = set(EXPERIMENT_SET_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for experiment sets." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for experiment sets." + ) class TestExperimentSetHasPermission: @@ -122,7 +122,9 @@ class TestExperimentSetReadActionHandler: PermissionTest("ExperimentSet", "published", "anonymous", Action.READ, True), PermissionTest("ExperimentSet", "private", "anonymous", Action.READ, False, 404), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_read_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_read_action helper function directly.""" @@ -171,7 +173,9 @@ class TestExperimentSetUpdateActionHandler: PermissionTest("ExperimentSet", "private", "anonymous", Action.UPDATE, False, 404), PermissionTest("ExperimentSet", "published", "anonymous", Action.UPDATE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_update_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_update_action helper function directly.""" @@ -218,7 +222,9 @@ class TestExperimentSetDeleteActionHandler: PermissionTest("ExperimentSet", "private", "mapper", Action.DELETE, False, 404), PermissionTest("ExperimentSet", "published", "mapper", Action.DELETE, False, 403), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_delete_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_delete_action helper function directly.""" @@ -265,7 +271,9 @@ class TestExperimentSetAddExperimentActionHandler: PermissionTest("ExperimentSet", "private", "anonymous", Action.ADD_EXPERIMENT, False, 404), PermissionTest("ExperimentSet", "published", "anonymous", Action.ADD_EXPERIMENT, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_experiment_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_experiment_action helper function directly.""" diff --git a/tests/lib/permissions/test_score_calibration.py b/tests/lib/permissions/test_score_calibration.py index 75120c6fa..e03be8944 100644 --- a/tests/lib/permissions/test_score_calibration.py +++ b/tests/lib/permissions/test_score_calibration.py @@ -47,9 +47,9 @@ def test_score_calibration_handles_all_actions() -> None: supported = set(SCORE_CALIBRATION_SUPPORTED_ACTIONS) unsupported = set(SCORE_CALIBRATION_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for score calibrations." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for score calibrations." + ) class TestScoreCalibrationHasPermission: diff --git a/tests/lib/permissions/test_score_set.py b/tests/lib/permissions/test_score_set.py index 2d5a0c497..fd004b90f 100644 --- a/tests/lib/permissions/test_score_set.py +++ b/tests/lib/permissions/test_score_set.py @@ -47,9 +47,9 @@ def test_score_set_handles_all_actions() -> None: supported = set(SCORE_SET_SUPPORTED_ACTIONS) unsupported = set(SCORE_SET_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for score sets." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for score sets." + ) class TestScoreSetHasPermission: @@ -124,7 +124,9 @@ class TestScoreSetReadActionHandler: PermissionTest("ScoreSet", "published", "anonymous", Action.READ, True), PermissionTest("ScoreSet", "private", "anonymous", Action.READ, False, 404), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_read_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_read_action helper function directly.""" @@ -171,7 +173,9 @@ class TestScoreSetUpdateActionHandler: PermissionTest("ScoreSet", "private", "anonymous", Action.UPDATE, False, 404), PermissionTest("ScoreSet", "published", "anonymous", Action.UPDATE, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_update_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_update_action helper function directly.""" @@ -216,7 +220,9 @@ class TestScoreSetDeleteActionHandler: PermissionTest("ScoreSet", "private", "mapper", Action.DELETE, False, 404), PermissionTest("ScoreSet", "published", "mapper", Action.DELETE, False, 403), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_delete_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_delete_action helper function directly.""" @@ -261,7 +267,9 @@ class TestScoreSetSetScoresActionHandler: PermissionTest("ScoreSet", "private", "anonymous", Action.SET_SCORES, False, 404), PermissionTest("ScoreSet", "published", "anonymous", Action.SET_SCORES, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_set_scores_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_set_scores_action helper function directly.""" @@ -308,7 +316,9 @@ class TestScoreSetPublishActionHandler: PermissionTest("ScoreSet", "private", "anonymous", Action.PUBLISH, False, 404), PermissionTest("ScoreSet", "published", "anonymous", Action.PUBLISH, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_publish_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_publish_action helper function directly.""" @@ -353,7 +363,9 @@ class TestScoreSetAddCalibrationActionHandler: PermissionTest("ScoreSet", "private", "anonymous", Action.ADD_CALIBRATION, False, 404), PermissionTest("ScoreSet", "published", "anonymous", Action.ADD_CALIBRATION, False, 401), ], - ids=lambda tc: f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}", + ids=lambda tc: ( + f"{tc.user_type}_{tc.entity_state}_{tc.action.value}_{'permitted' if tc.should_be_permitted else 'denied'}" + ), ) def test_handle_add_calibration_action(self, test_case: PermissionTest, entity_helper: EntityTestHelper) -> None: """Test _handle_add_calibration_action helper function directly.""" diff --git a/tests/lib/permissions/test_user.py b/tests/lib/permissions/test_user.py index d68f96964..b23cb132e 100644 --- a/tests/lib/permissions/test_user.py +++ b/tests/lib/permissions/test_user.py @@ -45,9 +45,9 @@ def test_user_handles_all_actions() -> None: supported = set(USER_SUPPORTED_ACTIONS) unsupported = set(USER_UNSUPPORTED_ACTIONS) - assert ( - supported.union(unsupported) == all_actions - ), "Some actions are not categorized as supported or unsupported for users." + assert supported.union(unsupported) == all_actions, ( + "Some actions are not categorized as supported or unsupported for users." + ) class TestUserHasPermission: diff --git a/tests/lib/test_odds_paths.py b/tests/lib/test_odds_paths.py index ce44546b4..9b7673c2c 100644 --- a/tests/lib/test_odds_paths.py +++ b/tests/lib/test_odds_paths.py @@ -66,9 +66,9 @@ def test_each_interval_is_exclusive(): upper_result = oddspaths_evidence_strength_equivalent(r[1]) assert lower_result == upper_result, f"Mismatch at interval {r}" - assert all( - result not in seen for result in [lower_result, upper_result] - ), f"Duplicate classification for ratio {r}" + assert all(result not in seen for result in [lower_result, upper_result]), ( + f"Duplicate classification for ratio {r}" + ) seen.add(lower_result) diff --git a/tests/lib/workflow/test_pipeline_definitions.py b/tests/lib/workflow/test_pipeline_definitions.py index c57875d8d..551e29a58 100644 --- a/tests/lib/workflow/test_pipeline_definitions.py +++ b/tests/lib/workflow/test_pipeline_definitions.py @@ -69,9 +69,9 @@ def test_pipeline_has_description(self, pipeline_name: str) -> None: @pytest.mark.parametrize("pipeline_name", list(PIPELINE_DEFINITIONS)) def test_pipeline_has_at_least_one_job(self, pipeline_name: str) -> None: """Every pipeline must define at least one job.""" - assert PIPELINE_DEFINITIONS[pipeline_name][ - "job_definitions" - ], f"Pipeline '{pipeline_name}' has no job definitions." + assert PIPELINE_DEFINITIONS[pipeline_name]["job_definitions"], ( + f"Pipeline '{pipeline_name}' has no job definitions." + ) # --- job field validity -------------------------------------------------- diff --git a/tests/routers/test_statistics.py b/tests/routers/test_statistics.py index 21ae62676..6618eaf76 100644 --- a/tests/routers/test_statistics.py +++ b/tests/routers/test_statistics.py @@ -96,12 +96,12 @@ def assert_statistic(desired_field_value, response): # Assert the GET request succeeded, that the desired field was present in the response, and that the number of times the field # appears in the database is 1. Tests calling this method should have created one score set containing the desired field. assert response.status_code == 200 - assert ( - desired_field_value in response.json() - ), f"Target accession statistic {desired_field_value} not present in statistic response." - assert ( - response.json()[desired_field_value] == 1 - ), f"Target accession statistic {desired_field_value} should appear on one (and only one) test score set." + assert desired_field_value in response.json(), ( + f"Target accession statistic {desired_field_value} not present in statistic response." + ) + assert response.json()[desired_field_value] == 1, ( + f"Target accession statistic {desired_field_value} should appear on one (and only one) test score set." + ) def add_query_param(url, query_name, query_value):