feat(framework): Enable FastAPI to manage SuperLinkLifespan#7453
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial FastAPI-based HTTP API scaffold for SuperLink/SuperNode within framework/py/flwr, along with dependency updates to support running these apps via uvicorn.
Changes:
- Add FastAPI app entrypoints for SuperLink and SuperNode, wiring in new router modules (health/control/runtime).
- Update
restoptional dependencies to include FastAPI (and bump Starlette/Uvicorn versions), and refreshuv.lock. - Add a short
FASTAPI.mdwith install/run instructions.
Critical issues
- Stub endpoints currently return HTTP 200 with a
"not_implemented"body; they should return an HTTP error status (e.g., 501) so clients/tooling don’t treat these as successful responses. (PR comments: IDs 003–005) FastAPI(..., version="1.32.0")is hard-coded in both apps and can drift fromflwr.__version__. (PR comments: IDs 001–002)
Simplicity/readability suggestions
restnow depends onfastapi[standard], which pulls in significant transitive tooling not used by the scaffold in this PR; consider depending on plainfastapi(or splitting into a separate extra) to keep the optional dependency footprint smaller. (PR comment: ID 006)
Consistency concerns
framework/FASTAPI.mdusesuv sync --all-extras; other framework docs recommenduv sync --locked ...for reproducible installs. (PR comment: ID 007)
Whether the PR should be split
No—these changes are all in service of introducing the FastAPI scaffold and its dependency/docs wiring.
Overall verdict
Changes are needed before approval.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/uv.lock | Locks new REST/FastAPI dependency set (FastAPI, updated Starlette/Uvicorn, transitive deps). |
| framework/pyproject.toml | Updates rest optional dependencies to include FastAPI and newer Starlette/Uvicorn pins. |
| framework/py/flwr/supercore/routers/__init__.py | Introduces core router package for shared endpoints. |
| framework/py/flwr/supercore/routers/health/__init__.py | Exposes the health router. |
| framework/py/flwr/supercore/routers/health/router.py | Adds /health endpoint router. |
| framework/py/flwr/superlink/main.py | Adds SuperLink FastAPI app and includes routers. |
| framework/py/flwr/superlink/routers/__init__.py | Introduces SuperLink router package. |
| framework/py/flwr/superlink/routers/control/__init__.py | Exposes the control router. |
| framework/py/flwr/superlink/routers/control/router.py | Adds stub control route(s). |
| framework/py/flwr/superlink/routers/runtime/__init__.py | Exposes the runtime router. |
| framework/py/flwr/superlink/routers/runtime/router.py | Adds stub runtime route(s). |
| framework/py/flwr/supernode/main.py | Adds SuperNode FastAPI app and includes routers. |
| framework/py/flwr/supernode/routers/__init__.py | Introduces SuperNode router package. |
| framework/py/flwr/supernode/routers/runtime/__init__.py | Exposes the runtime router. |
| framework/py/flwr/supernode/routers/runtime/router.py | Adds stub runtime route(s). |
| framework/FASTAPI.md | Adds install/run instructions for the new FastAPI apps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rest = [ | ||
| "starlette>=1.3.1,<1.4.0", | ||
| "uvicorn[standard]>=0.49.0,<0.50.0", | ||
| "fastapi[standard]>=0.138.0,<0.139.0", | ||
| ] |
github-actions
Bot
added
the
Maintainer
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ea37288e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if superlink_lifespan is not None and start_legacy_grpc: | ||
| # Temporary compatibility path: start the existing gRPC APIs from | ||
| # FastAPI lifespan | ||
| superlink_lifespan.startup() |
There was a problem hiding this comment.
In --enable-http-api compatibility mode, if SuperLinkLifespan.startup() raises after binding one or more legacy servers (for example a later Fleet/health port conflict or SuperExec spawn failure), this call happens before the try/finally, so superlink_lifespan.shutdown() is never invoked. The legacy CLI path explicitly shuts down after startup errors; the FastAPI path should do the same to avoid leaving gRPC ports or subprocesses alive in an in-process ASGI/test runner or other long-lived host.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5598042cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from flwr.common import log | ||
| from flwr.supercore.routers import health | ||
| from flwr.superlink.routers import runtime | ||
| from flwr.superlink.cli.flower_superlink import SuperLinkLifespan |
There was a problem hiding this comment.
This runtime import is only needed for the annotation, but it makes flwr.superlink.main import flwr.superlink.cli.flower_superlink while that CLI module also imports create_app from this module before SuperLinkLifespan is defined. With the FastAPI/Uvicorn dependencies installed, either flower-superlink or the documented uvicorn flwr.superlink.main:app path fails during module import with a circular ImportError; guard this with TYPE_CHECKING/a quoted annotation or move the dependency out of module scope.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66c2be71af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| FLWR_UPDATE_CHECK_SHOW_INTERVAL_SECONDS = 12 * 60 * 60 | ||
|
|
||
| # Constants for Uvicorn-backed API servers | ||
| UVICORN_DEFAULT_HOST = "127.0.0.1" |
There was a problem hiding this comment.
The new docs describe flower-superlink --insecure --enable-http-api as the compatibility mode being used in prod, but this default makes the FastAPI server listen only on loopback. In container or Kubernetes deployments where traffic reaches the pod over its pod IP/Service, 127.0.0.1 is unreachable from outside the process, unlike the existing SuperLink API defaults that bind to 0.0.0.0; operators following the documented command get no reachable HTTP API unless they know to add --host 0.0.0.0.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2258fa4b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ssl_keyfile=None if lifespan_config.insecure else lifespan_config.ssl_keyfile, | ||
| ssl_certfile=None if lifespan_config.insecure else lifespan_config.ssl_certfile, |
There was a problem hiding this comment.
When running HTTP-only without --insecure, for example flower-superlink --enable-http-api --disable-grpc-api --ssl-certfile server.pem --ssl-keyfile server.key, parsing exits in _obtain_superlink_certificates because it still requires the gRPC --ssl-ca-certfile before this disable_grpc_api branch is reached. In this path the HTTP server only passes ssl_keyfile and ssl_certfile to Uvicorn, so secure HTTP-only startup unnecessarily depends on an unrelated gRPC CA file; skip gRPC cert loading when legacy gRPC is disabled or validate HTTP TLS separately.
Useful? React with 👍 / 👎.
Merge after #7499