feat(framework): Add LinkState dependency for FastAPI#7561
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a shared FastAPI dependency for SuperLink routers to access the process-wide LinkState created by SuperLinkLifespan, along with unit tests validating the dependency behavior.
Changes:
- Introduce
get_linkstateFastAPI dependency that yields aLinkStatefromapp.state.superlink_lifespan.state_factory. - Add tests verifying successful yield and correct 503 error behavior when lifespan state is missing.
Critical issues
framework/py/flwr/superlink/routers/dependencies_test.pyusesRequest[State]in runtime-evaluated annotations withoutfrom __future__ import annotations, which can break module import withTypeErrorifRequestis not subscriptable (comment ID: 001).
Simplicity/readability suggestions
- None.
Consistency concerns
- None.
Whether the PR should be split
- No.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| framework/py/flwr/superlink/routers/dependencies.py | Adds get_linkstate dependency to provide LinkState to FastAPI routers via app lifespan state. |
| framework/py/flwr/superlink/routers/dependencies_test.py | Adds tests covering the dependency’s success path and missing-initialization error path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b24754500a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| _SuperLinkLifespanState | None, | ||
| getattr(request.app.state, "superlink_lifespan", None), | ||
| ) | ||
| if superlink_lifespan is None or superlink_lifespan.state_factory is None: |
There was a problem hiding this comment.
Support HTTP-only LinkState access
In the documented Next Mode (flower-superlink --enable-http-api --disable-grpc-api), _run_superlink_http_api leaves superlink_lifespan as None before calling create_app, so any FastAPI router using this dependency will always hit this 503 path instead of getting a LinkState. Since this helper is meant to back the HTTP API, it needs to obtain or initialize shared state independently of whether the legacy gRPC lifespan is started.
Useful? React with 👍 / 👎.
| superlink_lifespan = cast( | ||
| _SuperLinkLifespanState | None, | ||
| getattr(request.app.state, "superlink_lifespan", None), | ||
| ) | ||
| if superlink_lifespan is None or superlink_lifespan.state_factory is None: |
There was a problem hiding this comment.
Why do we use Protocol here? Is it because we can’t import SuperLinkLifespan in this module?
Also, would it be better to assign the LinkStateFactory directly to app.state, even though we can access it via app.state.superlink_lifespan.state_factory?
No description provided.