Skip to content

feat(framework): Add LinkState dependency for FastAPI#7561

Open
jafermarq wants to merge 4 commits into
mainfrom
add-fastapi-linkstate-dependency
Open

feat(framework): Add LinkState dependency for FastAPI#7561
jafermarq wants to merge 4 commits into
mainfrom
add-fastapi-linkstate-dependency

Conversation

@jafermarq

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings July 8, 2026 14:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_linkstate FastAPI dependency that yields a LinkState from app.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.py uses Request[State] in runtime-evaluated annotations without from __future__ import annotations, which can break module import with TypeError if Request is 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.

Comment thread framework/py/flwr/superlink/routers/dependencies_test.py Outdated
@github-actions github-actions Bot added the Maintainer Used to determine what PRs (mainly) come from Flower maintainers. label Jul 8, 2026
jafermarq and others added 3 commits July 8, 2026 16:57
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +36 to +40
superlink_lifespan = cast(
_SuperLinkLifespanState | None,
getattr(request.app.state, "superlink_lifespan", None),
)
if superlink_lifespan is None or superlink_lifespan.state_factory is None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fastapi-controlservicer Maintainer Used to determine what PRs (mainly) come from Flower maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants