Repository navigation
feat(controller): serve the API with FastAPI and Uvicorn - #665
Merged
Merged
Conversation
Replace Hypercorn with Uvicorn. Uvicorn runs in-process (workers 1), so the process Kubernetes signals is the one serving. At drain expiry Uvicorn cancels the request tasks and starts the lifespan hook without waiting for them, which keeps the shutdown design: the hook cancels the analysis RPCs, and the budget in controller_shutdown.py still sizes the pod grace period. Uvicorn re-raises the handled SIGTERM after a clean shutdown, so the exit status is -15.
KFilippopolitis
force-pushed
the
fix/controller-modernization-wave3
branch
3 times, most recently
from
November 11, 2026 16:31
29ef6b7 to
b4239ec
Compare
Base automatically changed from
fix/controller-modernization-wave2
to
master
November 11, 2026 16:32
KFilippopolitis
force-pushed
the
fix/controller-modernization-wave3
branch
2 times, most recently
from
November 11, 2026 17:22
7f98679 to
f3f594d
Compare
KFilippopolitis
marked this pull request as ready for review
November 11, 2026 17:22
ThanKarab
reviewed
Nov 12, 2026
ThanKarab
left a comment
Member
There was a problem hiding this comment.
Great work!
Please improve a bit the endpoint validations to be FastAPI native and NOT quart or test compatible.
Member
|
GPT6.1Sol XHigh Review: |
Collaborator
Author
|
Both findings of the GPT review are addressed:
The branch was rewritten into one commit per task, so the earlier commit hashes in this thread no longer exist. Contract changes, also listed in the PR description: a malformed body is answered with Known open point: the |
ThanKarab
self-requested a review
November 12, 2026 11:51
ThanKarab
approved these changes
Nov 12, 2026
KFilippopolitis
force-pushed
the
fix/controller-modernization-wave3
branch
from
November 12, 2026 11:56
8ab59c8 to
84d2438
Compare
Map each controller error to its HTTP status and log level in ERROR_RESPONSES and register one handler per entry, instead of a hand-written handler for each error. Responses are unchanged.
The /analysis route only reads the body. analysis_service.execute_analysis parses the request, validates it, and runs the strategy, so the route no longer holds the orchestration.
Replace Quart with FastAPI. Hypercorn still serves. Routes, status codes, content types, and bodies stay the same. The OpenAPI and docs routes are disabled.
KFilippopolitis
force-pushed
the
fix/controller-modernization-wave3
branch
from
November 12, 2026 12:03
84d2438 to
0fdb548
Compare
…pped A request cancelled at drain expiry runs its cleanup through run_blocking after the shutdown hook cleared the pool, which starts a new pool with the same thread prefix. The join counted that pool's idle thread, waited the whole SHUTDOWN_RPC_CANCEL_TIMEOUT and warned that analysis threads were still running. It made test_uvicorn_shutdown.py fail under load. The hook now snapshots the threads under the state guard and joins only those.
The path was relative to the working directory, so `inv start-controller` found the file only when it ran from the repository root.
The controller tests used a client class that copied the Quart response methods (get_data, content_type), reached through app.test_client. They now use httpx.AsyncClient over ASGITransport and read response.text and response.headers. The client stays async: several tests send concurrent requests on one event loop, for example /livez while an analysis blocks. The client module moves from exaflow/controller/http into the tests, and app.test_client is removed.
POST /analysis declares AnalysisRequestDTO as its body and POST /flower/result declares its body with Body(), so FastAPI parses and validates them. analysis_service.run_analysis takes the DTO. It no longer takes the callables that tests patched on the route module; tests patch the service module. A body that is not JSON or does not match the model is answered with FastAPI's status, 422. The body stays plain text like the other errors, starts with "Request malformed:" and lists the invalid fields. Breaking: this case was answered with 400 and the message "Analysis execution request malformed:". platform-backend must accept 422 from the controller before this is deployed.
The specification routes return their DTOs. The analysis result is served as application/json. /wla, /healthcheck and /livez return an empty response. Error responses are text/plain. The text/html responses kept from Quart, and the html_response helper, are removed. The analysis route still builds a Response: each algorithm has its own result model and the strategy returns it already serialized. Breaking: the Content-Type of the specification, analysis and error responses changed from text/html.
Uvicorn does not limit the request body, so a large POST was buffered whole before the analysis concurrency limit applied. Quart limited it to 16 MiB. A middleware counts the body as the route reads it and answers 413 above the limit. It counts received bytes, not Content-Length, so chunked uploads are limited too.
KFilippopolitis
force-pushed
the
fix/controller-modernization-wave3
branch
from
November 12, 2026 13:17
0fdb548 to
95a2ee9
Compare
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.
Stacked on #664. Replaces Quart and Hypercorn with FastAPI and Uvicorn. Data bodies are unchanged. After review, a malformed body is answered with 422 and content types changed (see Compatibility). Needs Medical-Informatics-Platform/platform-backend#55.
Commits
One error table (
refactor(controller)): the hand-written handlers are oneERROR_RESPONSEStable of(status, log level), with one handler registered per entry.ControllerShuttingDownErrorstays 503.InsufficientDataErrorstill returns its fixed message.HTTPStatusCode.UNEXPECTED_ERRORand the commented generic handler are gone.Analysis service (
refactor(controller)):exaflow.controller.services.analysis_service.run_analysisowns validation, strategy selection,analysis_slot(), andexecute(). It takes the validatedAnalysisRequestDTO.FastAPI (
feat(controller)): Quart is replaced with FastAPI. Hypercorn still serves. Routes, status codes, content types, and bodies stay the same. The OpenAPI and docs routes are disabled.Uvicorn (
chore(controller)): Hypercorn is replaced with Uvicorn. See below. FastAPI under Hypercorn does not exit after SIGTERM while an analysis runs, so serving changes in this commit.Analysis thread join (
fix(controller)): the shutdown hook joins only the analysis threads of the pool it stops.Log config path (
fix(tasks)):tasks.pypasses the Uvicorn log config by absolute path.Test client (
test(controller)): the tests call the app through a plainhttpx.AsyncClient. The client class with the Quart response methods andapp.test_clientare removed.Model validation (
refactor(controller)):/analysisdeclaresAnalysisRequestDTOas its body. A body that fails validation is answered with 422. The service takes the DTO.Response types (
refactor(controller)): the specification routes return their DTOs. Data isapplication/json, errors aretext/plain.html_responseis removed.Body limit (
feat(controller)): a middleware answers 413 for a request body over 16 MiB.Commits 5 to 10 are the review changes.
Each of the first two commits passes the standalone suite on its own. Commit 3 (FastAPI under Hypercorn) keeps the HTTP contract tests green.
FastAPI
exaflow.controller.http.appis a FastAPI app withdocs_url=None,redoc_url=None, andopenapi_url=None, so/docs,/redoc, and/openapi.jsonreturn 404./analysisdeclaresAnalysisRequestDTOas its body, so FastAPI validates it.RequestValidationError(a body that is not JSON or does not match the model) is served with FastAPI's status, 422, and a plain-text body like the other errors. The/specifications/*routes return their DTOs. A middleware limits the request body to 16 MiB and answers 413 above it. The lifespan keeps the startup and shutdown order: start the landscape and the analysis pool; on shutdown, cancel the analysis RPCs, stop the landscape, and shut the pool down infinally. The Flower routes are ported unchanged.fastapi0.115.14 andstarlette0.46.2 are added,httpx0.28.1 for tests. Quart is removed.Uvicorn
Image command, exec form:
python -m uvicorn exaflow.controller.http.app:app --host 0.0.0.0 --port 5000 --workers 1 --timeout-graceful-shutdown 120 --log-config exaflow/controller/http/log_config.json--workers 1serves in-process on Uvicorn 0.32.1 (a supervisor starts only whenworkers > 1), so the process Kubernetes signals is the one serving.--timeout-graceful-shutdown 120is the drain. At drain expiry Uvicorn cancels the request tasks still running and starts the lifespan shutdown hook without waiting for them. A cancelled request still waits for its analysis thread (run_blocking), and the hook callsbegin_controller_shutdown(), which cancels the registered RPCs. This is the same shutdown design as under Hypercorn. There is no signal-handler chaining, so a client disconnect during the drain still cancels nothing.exaflow/controller/services/controller_shutdown.py.exaflow/controller/http/server_config.pyholds onlyworkersandgraceful_timeout.tasks.pyrepeats those two values, with noexaflowimport, and a test keeps them equal.hypercornandhypercorn_config.pyare removed.tasks.pystarts and stops Uvicorn. The Helm comments point at the new modules.Compatibility
/docs,/redoc, and/openapi.jsonare not served. Changed:/specifications/*and/analysisresults areapplication/json, nottext/html.text/plain, nottext/html.Request malformed:and lists the invalid fields. It no longer echoes the request. 400 stays for the other request errors./analysisand/flower/resultneedContent-Type: application/json.worker_tasks_timeout. The chart comments changed. This needs the human review required bykubernetes/AGENTS.md.uv.lockupdated.Testing
pytest -q -n auto --dist worksteal -m "not smpc" tests/standalone_testson each commit, each with the dependencies of its own lockfile: 2466, 2468, and 2472 passed, 30 warnings.test_uvicorn_shutdown.pystarts a real Uvicorn process with the controller settings (drain shortened to 3 s). It checks that an analysis running at drain expiry has its RPC cancelled, that its cleanup still runs, and that a client disconnect during the drain does not cancel other analyses. It passed three runs in a row.pytest -q tests/standalone_tests/controller, 345 passed. The 8 Helm-rendering tests intest_controller_probes.pywere not rerun locally.Standalone Testsworkflow stopped with exit code 143 on an earlier head of the review changes, after its last tests hung. Under investigation.Needs human review
/livezfails once the listen socket closes. That relies on kubelet stopping liveness probes of a terminating pod.log_config.jsonis static. StartupdictConfigthen applies the federation- and node-aware format fromloggers.py.Rollback
Revert the PR, and roll back the controller image together with the chart.