Repository navigation
Conversation
FastAPI runs async def handlers on the event loop, and most of ours ran synchronous SQLAlchemy queries there, so one slow query stalled every in-flight request on its process. The publication fetchers were async in name only: they made synchronous HTTP calls to NCBI, bioRxiv and Crossref on the loop, with no timeout. Routes that use the database are now def, so FastAPI runs them in its thread pool. The helpers they awaited, which never awaited anything real, are sync. MONDO and NCBI Taxonomy use a sync httpx client, and routes queue jobs through enqueue_pipeline_for_score_set_from_thread, which writes the pipeline records in the calling thread and hands only the ARQ call to the loop. The two upload routes stay async to read the multipart form and run everything after it, including the CSV rebuild and S3 upload, in the thread pool. Making them def would mean declaring every form field, which changes the OpenAPI schema. The schema is unchanged. Disease search could be async, since it only calls OLS, but it's def to keep one client style in lib/mondo_ols.py; its comment says when to revisit that. Tests that were async only because these helpers were are now sync, with the same assertions.
Walks the app's routes and fails when an async endpoint or dependency takes a session from get_db, so sync database work can't drift back onto the event loop. A def dependency that uses the session passes, since it runs in the thread pool. FastAPI keeps each included router behind a wrapper rather than copying its routes into app.routes, so the walk follows original_router and asserts it finds at least 150 routes; without that, a walk that saw nothing would pass. It fails on the code before the conversion and on a route or dependency made async again.
Replaces "never write an async def with no real await" with the rule the conversion settled on: def by default, async only where the code can't work without an await, database work kept out of it, and a comment saying why. The routes rule names the guard test, what may go on its allowlist, and the thread-pool variant of the pipeline kickoff. Expect: agents write new routes and helpers as def, and add a comment giving the reason whenever they write an async route.
Coverage Report for CI Build 40032530162Coverage decreased (-0.07%) to 90.337%Details
Uncovered Changes
Coverage Regressions12 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
4 tasks
bencap
marked this pull request as ready for review
October 23, 2026 23:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Most
async defroutes ran synchronous SQLAlchemy queries on the event loop, so one slow query stalled every in-flight request on its process. The publication fetchers wereasyncin name only: they made synchronous HTTP calls to NCBI, bioRxiv and Crossref on the loop, with no timeout. This moves that work into FastAPI's thread pool and adds a test that keeps it there.def. Only 4 stayasync: the two upload routes, and the AlphaFold and ORCID proxies, which never touch the database. Each one says why in a comment.POST /score-sets/{urn}/variants/dataandPATCH /score-sets-with-variants/{urn}await only the multipart form. Everything after it, including the CSV rebuild and S3 upload that used to run on the loop, runs in the thread pool. Making themdefwould mean declaring every form field, which changes the OpenAPI schema.asyncwithout awaiting anything real are sync. MONDO and NCBI Taxonomy use a synchttpxclient.enqueue_pipeline_for_score_set_from_threadwrites the pipeline records in the calling thread and runs only the ARQ call on the loop. The async version is unchanged for scripts.def, though it only calls OLS and could beasync. Its comment says when to switch.AGENTS.mdand.claude/rules/routes.mdstate the rule: writedefby default, and useasynconly where the code needs an await.The OpenAPI schema is unchanged, so the ui needs no update. No migrations or settings.
Closes #950
Tests
tests/routers/test_async_routes.pyfails when anasyncendpoint or dependency takes a session fromget_db. It flags dozens of routes on the code before this change, and it caught a route and a dependency that were temporarily madeasyncagain.enqueue_pipeline_for_score_set_from_threadin an AnyIO worker thread: success, cleanup when the enqueue fails, and a call from outside a worker thread.asynconly because the helpers they called were. They're sync now, with the same assertions, and the MONDO fakes now stand in forhttpx.Client.bin/checkand the PR suite (bin/t tests/ -n 6 -m "not network and not slow"): 5,438 passed, 22 skipped. Also ran the MONDO network tests against live OLS: 5 passed.app.openapi()before and after: identical.Checklist
release-<year>.<n>.0branch otherwise; never
mainmavedb-uiPR regeneratessrc/schema/openapi.d.ts