feat: add snapshot in py server#24
Open
kerthcet wants to merge 4 commits into
Open
Conversation
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces snapshot operations (create/restore/list/find/get/delete) to the SandD daemon and exposes them through the Python server API, backed by new protocol message variants and expanded end-to-end tests.
Changes:
- Added snapshot message types to the WebSocket protocol and implemented snapshot request handling in the daemon.
- Added Python-facing snapshot APIs and a
SnapshotInfomodel to represent snapshot metadata. - Updated snapshot manager/types naming and expanded E2E coverage for snapshot workflows.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/lib.rs | Adds PyO3-exposed snapshot methods that send snapshot protocol messages and await responses. |
| sandd/src/snapshot/types.rs | Renames snapshot field to workspace (from workspace_path). |
| sandd/src/snapshot/manager.rs | Renames find_by_tag to find_snapshot_by_tag and changes get_snapshot to return SnapshotInfo. |
| sandd/src/protocol.rs | Adds snapshot-related protocol message variants. |
| sandd/src/main.rs | Wires SnapshotManager into the daemon message handler and implements snapshot operations. |
| python/sandd/server.py | Adds Python API methods for snapshot operations and maps raw results into SnapshotInfo. |
| python/sandd/models.py | Introduces SnapshotInfo Python model (including timestamp handling). |
| python/tests/test_e2e.py | Adds extensive E2E tests for snapshot create/list/find/get/restore/delete behaviors. |
| examples/snapshot_simple.rs | Updates example code to the renamed find_snapshot_by_tag API. |
| docs/proposals/SNAPSHOTS.md | Updates the proposal doc with snapshot protocol integration details and renamed APIs. |
Comments suppressed due to low confidence (1)
docs/proposals/SNAPSHOTS.md:199
- The proposal still documents
get_snapshotreturning a fullSnapshot, but the implementation now returnsSnapshotInfo(seesandd/src/snapshot/manager.rs). The doc should match the implemented API.
/// Get snapshot by ID
pub async fn get_snapshot(&self, id: &str) -> Result<Snapshot>;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+373
to
+377
| let msg = Message::CreateSnapshot { | ||
| request_id: request_id.clone(), | ||
| workspace, | ||
| message, | ||
| tags, |
Comment on lines
+474
to
+475
| let snapshots: Vec<serde_json::Value> = serde_json::from_str(&result.stdout) | ||
| .map_err(|e| PyRuntimeError::new_err(format!("Invalid snapshot list: {}", e)))?; |
| pub message: String, | ||
| pub tags: Vec<String>, | ||
| pub workspace_path: PathBuf, | ||
| pub workspace: PathBuf, |
Comment on lines
+146
to
+148
| self.id = id | ||
| self.created_at = datetime.fromtimestamp(created_at) | ||
| self.message = message |
Comment on lines
+231
to
+234
| request_id: String, | ||
| snapshot_id: String, // Snapshot ID or tag name | ||
| destination: String, // Path to restore to | ||
| }, |
Comment on lines
+265
to
268
| SnapshotDetails { | ||
| request_id: String, | ||
| snapshot: Snapshot, // Full snapshot metadata | ||
| }, |
Comment on lines
+388
to
+392
| if let Some(snapshot_id) = result.stdout.split_whitespace().next() { | ||
| Ok(snapshot_id.to_string()) | ||
| } else { | ||
| Err(PyRuntimeError::new_err("Invalid snapshot response")) | ||
| } |
Signed-off-by: kerthcet <kerthcet@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
Which issue(s) this PR fixes
Fixes #
Special notes for your reviewer
Does this PR introduce a user-facing change?