feat(framework): Add Automation Control implementation#7567
feat(framework): Add Automation Control implementation#7567danielnugraha wants to merge 14 commits into
Conversation
…o' into add-automation-proto
![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: 5573704d7f
ℹ️ 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".
| """Return the automation start timestamp.""" | ||
| if request.HasField("start_at"): | ||
| try: | ||
| return datetime.fromisoformat(request.start_at) |
There was a problem hiding this comment.
When start_at includes a non-UTC offset, fromisoformat preserves that offset and the value is stored as-is; the due queries then compare the serialized timestamp text against a UTC due_before, so a due automation like 2026-07-10T10:00:00+02:00 can be missed when polling at 2026-07-10T09:00:00+00:00. Normalize parsed values to UTC (or reject non-UTC inputs) before storing/comparing so automations with valid ISO offsets run when due.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a first implementation of the Control API surface for automations (Start/List/Stop) and wires it through the servicer into LinkState/CoreState, including persistence support in both SQL and in-memory CoreState implementations.
Changes:
- Implement
StartAutomation,ListAutomations, andStopAutomationincontrol_handlersand delegate to them fromControlServicer - Add CoreState automation persistence APIs (
store_automation,list_automations,stop_automation) for SQL and in-memory backends - Add unit tests covering automation start/list/stop behavior and corestate filtering/stopping semantics
Critical issues
_parse_start_atcurrently rejects RFC3339 timestamps with a trailingZ(common in this codebase viaisoformat8601_utc), which can cause valid StartAutomation requests to fail.fixed_interval/max_runsallow0without validation, which can store unusable automation records.
Simplicity/readability suggestions
- None blocking.
Consistency concerns
- None blocking beyond the issues above.
Whether the PR should be split
- No.
Brief overall verdict
- Functionality is broadly in place and well-tested, but the timestamp parsing and input validation issues should be addressed before approval.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/py/flwr/superlink/servicer/control/control_servicer.py | Delegates automation RPCs to handler implementations |
| framework/py/flwr/superlink/servicer/control/control_servicer_test.py | Adds ControlServicer tests for automation start/list/stop |
| framework/py/flwr/superlink/servicer/control/control_handlers.py | Implements Start/List/Stop automation handlers and shared run-input resolution |
| framework/py/flwr/supercore/corestate/sql_corestate.py | Adds SQL-backed automation store/list/stop implementations |
| framework/py/flwr/supercore/corestate/in_memory_corestate.py | Adds in-memory automation store/list/stop implementations |
| framework/py/flwr/supercore/corestate/corestate.py | Extends CoreState interface with automation methods |
| framework/py/flwr/supercore/corestate/corestate_test.py | Adds CoreState tests for automation storage, filtering, and stopping |
| framework/py/flwr/supercore/constant.py | Introduces automation status constants |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _parse_start_at(request: StartAutomationRequest) -> datetime: | ||
| """Return the automation start timestamp.""" | ||
| if request.HasField("start_at"): | ||
| try: | ||
| return datetime.fromisoformat(request.start_at) | ||
| except ValueError as e: | ||
| raise FlowerError( | ||
| ApiErrorCode.INVALID_RUN_CONFIG, | ||
| f"Invalid automation start_at value: {request.start_at}", | ||
| ) from e | ||
| return now() |
| next_run_at = _parse_start_at(request) | ||
| fixed_interval = ( | ||
| request.fixed_interval if request.HasField("fixed_interval") else None | ||
| ) | ||
| remaining_runs = ( | ||
| request.max_runs | ||
| if request.HasField("max_runs") | ||
| else 1 | ||
| if fixed_interval is None | ||
| else None | ||
| ) |
github-actions
Bot
added
the
Maintainer
Add Control API implementation for
ListAutomations,StartAutomation,StopAutomation.