Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the IPC protocol to use scoped/namespaced message enums (Hub/Storage/Scheduler/Executor/System) and updates the various processes (CLI, Scheduler, Storage, Executor) to send/handle the newly scoped payloads.
Changes:
- Introduces scoped protocol message enums (
HubMessage,StorageMessage,SchedulerMessage,ExecutorMessage,SystemMessage) and wraps them underMessagePayload. - Updates Storage/Scheduler/Executor/CLI codepaths to construct and match against the scoped payloads.
- Updates Unix socket transport tests to use the new
SystemMessageping/pong structure.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/storage/src/lib.rs | Updates Storage IPC handling to match on scoped payload enums and return scoped responses. |
| src/scheduler/src/lib.rs | Updates Scheduler request/response matching to scoped payloads and sends scoped messages. |
| src/ipc/src/transport/unix_socket.rs | Adjusts transport tests for SystemMessage ping/pong under scoped payloads. |
| src/ipc/src/protocol.rs | Defines scoped message enums and updates Message::new to accept Into<MessagePayload>. |
| src/cli/src/server/api/v0/jobs/retrieve.rs | Switches job retrieval IPC request/response to scoped messages. |
| src/cli/src/server/api/v0/jobs/create.rs | Switches job creation IPC request/response to scoped messages. |
| src/cli/src/server/api/v0/health/retrieve.rs | Switches health checks to SystemMessage::Ping. |
| src/cli/src/process/hub.rs | Switches hub health checks to SystemMessage::Ping. |
| src/cli/src/process/executor.rs | Switches executor IPC messages to scoped scheduler/system/executor messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub enum HubMessage { | ||
| StoreJob(Box<Job>), | ||
| QueryJobs(JobQuery), | ||
| UpdateJobStatus(Uuid, JobStatus), | ||
| } |
There was a problem hiding this comment.
HubMessage::UpdateJobStatus is defined here but (in this PR) there’s no corresponding receiver-side handling in Storage (nor backend support), and the only send sites are Scheduler→Storage. This makes the variant effectively a no-op at runtime; either implement end-to-end handling or remove/relocate it to the appropriate message scope.
| .send(Message::new( | ||
| IPC_SENDER_SCHEDULER, | ||
| ProcessType::Storage, | ||
| MessagePayload::UpdateJobStatus(job_id, JobStatus::Pending), | ||
| MessagePayload::Hub(HubMessage::UpdateJobStatus(job_id, JobStatus::Pending)), | ||
| )) |
There was a problem hiding this comment.
HubMessage::UpdateJobStatus is being sent to ProcessType::Storage, but src/storage/src/lib.rs::handle_message has no match arm for HubMessage::UpdateJobStatus (it will fall through to _ => None). As a result, these status updates are silently dropped and job status won’t be updated as intended; either add Storage-side handling (and a backend method) or send a message type that Storage actually processes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.