feat(framework): Add flwr-connector#7029
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Javier <[email protected]>
danielnugraha
requested review from
danieljanes,
panh99 and
tanertopal
as code owners
There was a problem hiding this comment.
Pull request overview
This PR introduces new process-isolated CLI commands (flwr-agentapp, flwr-connector, flwr-model) and corresponding stub runtimes, along with telemetry/exit integration and tests.
Changes:
- Add new
flwr-*CLI entrypoints (agentapp/connector/model) with argument parsing and wiring to runtimes. - Add stub runtime implementations for AgentApp/ConnectorApp/ModelApp plus unit tests.
- Extend telemetry
EventTypeand exit telemetry auto-detection for the new CLIs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/pyproject.toml | Adds a new console script entry for flwr-agentapp. |
| framework/py/flwr/supercore/cli/flwr_model_test.py | Tests CLI parsing and argument forwarding for flwr-model. |
| framework/py/flwr/supercore/cli/flwr_model.py | Implements flwr-model CLI wrapper and arg parser. |
| framework/py/flwr/supercore/cli/flwr_connector_test.py | Tests CLI parsing and argument forwarding for flwr-connector. |
| framework/py/flwr/supercore/cli/flwr_connector.py | Implements flwr-connector CLI wrapper and arg parser. |
| framework/py/flwr/supercore/cli/flwr_agentapp_test.py | Tests CLI parsing and argument forwarding for flwr-agentapp. |
| framework/py/flwr/supercore/cli/flwr_agentapp.py | Implements flwr-agentapp CLI wrapper and arg parser. |
| framework/py/flwr/supercore/cli/init.py | Exposes the new CLI callables via __all__. |
| framework/py/flwr/supercore/agent/run_model_test.py | Tests stub exit behavior and telemetry details for run_model. |
| framework/py/flwr/supercore/agent/run_model.py | Adds stub ModelApp runtime that exits with “not implemented yet”. |
| framework/py/flwr/supercore/agent/run_connector_test.py | Tests stub exit behavior and telemetry details for run_connector. |
| framework/py/flwr/supercore/agent/run_connector.py | Adds stub ConnectorApp runtime that exits with “not implemented yet”. |
| framework/py/flwr/supercore/agent/run_agentapp_test.py | Tests stub exit behavior and telemetry details for run_agentapp. |
| framework/py/flwr/supercore/agent/run_agentapp.py | Adds stub AgentApp runtime that exits with “not implemented yet”. |
| framework/py/flwr/supercore/agent/init.py | Exposes the new runtime functions via __all__. |
| framework/py/flwr/common/telemetry.py | Adds new telemetry event types for agentapp/model/connector. |
| framework/py/flwr/common/exit/exit.py | Maps the new CLI names to telemetry “leave” events on flwr_exit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| flower-superexec = "flwr.supercore.cli:flower_superexec" | ||
| flwr-serverapp = "flwr.server.serverapp:flwr_serverapp" | ||
| flwr-clientapp = "flwr.supernode.cli:flwr_clientapp" | ||
| flwr-agentapp = "flwr.supercore.cli:flwr_agentapp" |
There was a problem hiding this comment.
Only flwr-agentapp is added as a console script here, but this PR also introduces flwr-connector and flwr-model CLIs. Without corresponding [project.scripts] entries, those commands won’t be installable/invokable via the package. Also, the PR title mentions flwr-connector, but the only new script exposed is flwr-agentapp.
| "--serverappio-api-address", | ||
| default=SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS, | ||
| type=str, | ||
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." |
There was a problem hiding this comment.
The help= string concatenation is missing a separating space/newline between sentences, so argparse --help will render as ...).By default, .... Consider adding an explicit space at the end of the first literal (or using a single combined literal) for readability.
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." | |
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name). " |
| "--serverappio-api-address", | ||
| default=SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS, | ||
| type=str, | ||
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." |
There was a problem hiding this comment.
The help= string concatenation is missing a separating space/newline between sentences, so argparse --help will render as ...).By default, .... Consider adding an explicit space at the end of the first literal (or using a single combined literal) for readability.
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." | |
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name). " |
| "--serverappio-api-address", | ||
| default=SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS, | ||
| type=str, | ||
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." |
There was a problem hiding this comment.
The help= string concatenation is missing a separating space/newline between sentences, so argparse --help will render as ...).By default, .... Consider adding an explicit space at the end of the first literal (or using a single combined literal) for readability.
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name)." | |
| help="Address of SuperLink's ServerAppIo API (IPv4, IPv6, or a domain name). " |
![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: a785c57007
ℹ️ 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".
| flower-superexec = "flwr.supercore.cli:flower_superexec" | ||
| flwr-serverapp = "flwr.server.serverapp:flwr_serverapp" | ||
| flwr-clientapp = "flwr.supernode.cli:flwr_clientapp" | ||
| flwr-agentapp = "flwr.supercore.cli:flwr_agentapp" |
There was a problem hiding this comment.
The commit adds and exports flwr.supercore.cli.flwr_connector and flwr.supercore.cli.flwr_model, but [project.scripts] only registers flwr-agentapp. This means packaged installs will not generate flwr-connector/flwr-model executables, so the new commands are not runnable from the CLI even though their command modules were added. Please add script entries for both commands in pyproject.toml to make the feature usable.
Useful? React with 👍 / 👎.
github-actions
Bot
added
the
Maintainer
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f77aa1358f
ℹ️ 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.constant import SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS | ||
| from flwr.common.exit import ExitCode, flwr_exit | ||
| from flwr.common.logger import log, mirror_output_to_queue, restore_output | ||
| from flwr.supercore.agent.run_agentapp import run_agentapp |
There was a problem hiding this comment.
flwr_agentapp imports flwr.supercore.agent.run_agentapp, but this commit only adds runtime modules under flwr/supercore/executors/ and does not add a flwr/supercore/agent package (same pattern in flwr_connector.py and flwr_model.py). Because flwr.supercore.cli.__init__ eagerly imports these modules, loading flwr.supercore.cli will raise ModuleNotFoundError, which also breaks existing entry points like flower-superexec that depend on that package import.
Useful? React with 👍 / 👎.
| from flwr.common import EventType | ||
| from flwr.common.exit import ExitCode | ||
|
|
||
| run_agentapp_module = importlib.import_module("flwr.supercore.agent.run_agentapp") |
There was a problem hiding this comment.
This test imports flwr.supercore.agent.run_agentapp, but the runtime added in this commit is located in flwr.supercore.executors.run_agentapp and no agent package exists. As a result, test collection fails with ModuleNotFoundError before any assertions run (same issue is present in the connector/model executor tests), so the added tests currently do not validate the new behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 278d92d98c
ℹ️ 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_exit( | ||
| ExitCode.SERVERAPP_EXCEPTION, | ||
| "`flwr-agentapp` is not implemented yet.", | ||
| event_type=EventType.FLWR_AGENTAPP_RUN_LEAVE, | ||
| event_details={"success": False}, |
There was a problem hiding this comment.
This new runtime only emits a *_RUN_LEAVE event in flwr_exit, but it never emits the corresponding FLWR_AGENTAPP_RUN_ENTER (and the same pattern appears in run_connector and run_model). Existing runtimes such as run_clientapp/run_serverapp publish enter events, so these new commands will produce incomplete telemetry sessions and skew any start/finish or success-rate metrics for these executors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea9f3f75fd
ℹ️ 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 EventType | ||
| from flwr.common.exit import ExitCode | ||
|
|
||
| run_connector_module = importlib.import_module("flwr.supercore.agent.run_connector") |
There was a problem hiding this comment.
Update this import to the module path introduced by the commit (flwr.supercore.executors.run_connector). As written, flwr.supercore.agent.run_connector does not exist in this tree, so pytest fails during test collection with ModuleNotFoundError and the new executor test never runs.
Useful? React with 👍 / 👎.
Issue
Description
Related issues/PRs
Proposal
Explanation
Checklist
#contributions)Any other comments?