-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor(framework) Rename Heartbeat to SendNodeHeartbeat
#5315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the heartbeat functionality to adopt a "node heartbeat" naming convention. The changes update method names, REST endpoint paths, and corresponding gRPC and client interfaces to reflect the new naming.
- Renamed methods such as acknowledge_heartbeat to acknowledge_node_heartbeat.
- Updated REST endpoints and gRPC service methods from "heartbeat" to "send_node_heartbeat".
- Adjusted client and test modules to call the new node heartbeat methods.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| framework/py/flwr/server/superlink/linkstate/in_memory_linkstate.py | Renamed the heartbeat acknowledgment method to acknowledge_node_heartbeat. |
| framework/py/flwr/server/superlink/fleet/rest_rere/rest_api.py | Updated REST endpoint and handler from heartbeat to send_node_heartbeat. |
| framework/py/flwr/server/superlink/fleet/message_handler/message_handler.py | Renamed heartbeat function to send_node_heartbeat for improved clarity. |
| Other files (grpc, client, tests, proto) | Adjusted method names, route paths, and request/response types to maintain consistent naming across the framework. |
Comments suppressed due to low confidence (3)
framework/py/flwr/server/superlink/linkstate/in_memory_linkstate.py:542
- [nitpick] Consider updating the docstring for 'acknowledge_node_heartbeat' to clearly state that it acknowledges receipt of a node heartbeat, ensuring consistency with the new naming convention.
def acknowledge_node_heartbeat(
framework/py/flwr/server/superlink/fleet/rest_rere/rest_api.py:171
- [nitpick] Ensure that the updated endpoint path and function name 'send_node_heartbeat' are consistently documented throughout the codebase and aligned with the overall node heartbeat concept.
Route("/api/v0/fleet/send-node-heartbeat", send_node_heartbeat, methods=["POST"]),
framework/py/flwr/server/superlink/fleet/message_handler/message_handler.py:78
- [nitpick] Verify that all references to the renamed 'send_node_heartbeat' function are updated consistently, and update any inline comments if needed to match the new naming convention.
def send_node_heartbeat(
Heartbeat to SendNodeHeartbeat
framework/py/flwr/server/superlink/fleet/grpc_adapter/grpc_adapter_servicer.py
Outdated
Show resolved
Hide resolved
framework/py/flwr/server/superlink/fleet/grpc_rere/fleet_servicer.py
Outdated
Show resolved
Hide resolved
framework/py/flwr/server/superlink/fleet/grpc_rere/server_interceptor_test.py
Outdated
Show resolved
Hide resolved
framework/py/flwr/server/superlink/fleet/message_handler/message_handler.py
Outdated
Show resolved
Hide resolved
chongshenng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @panh99, thanks for the refactoring! Couple of nits about renaming and one suggestion to move the introduction of heartbeat.proto to another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR renames all occurrences of the heartbeat functions to explicitly include “node” in their names to improve clarity.
- Renames acknowledge_heartbeat to acknowledge_node_heartbeat in the linkstate implementations and tests.
- Updates associated gRPC and REST endpoints, message handler functions, and client-side functions accordingly.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| framework/py/flwr/server/superlink/linkstate/sqlite_linkstate.py | Renamed method in SQLite based LinkState to reflect node heartbeat. |
| framework/py/flwr/server/superlink/linkstate/linkstate_test.py | Updated test method names and documentation to reflect renaming. |
| framework/py/flwr/server/superlink/linkstate/linkstate.py | Updated abstract method declaration for node heartbeat. |
| framework/py/flwr/server/superlink/linkstate/in_memory_linkstate.py | Renamed method in in-memory LinkState to reflect node heartbeat. |
| framework/py/flwr/server/superlink/fleet/rest_rere/rest_api.py | Updated REST API endpoints and handler references. |
| framework/py/flwr/server/superlink/fleet/message_handler/message_handler.py | Renamed handler function for node heartbeat. |
| framework/py/flwr/server/superlink/fleet/grpc_rere/* (several files) | Updated gRPC method and stub names for node heartbeat. |
| framework/py/flwr/client/rest_client/connection.py | Updated REST client endpoints and heartbeat function usage. |
| framework/py/flwr/client/grpc_rere_client/* (several files) | Updated gRPC client method references for node heartbeat. |
| framework/proto/flwr/proto/fleet.proto | Updated proto definitions for node heartbeat messages. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
chongshenng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.