Conversation
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Manual Testing
|
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the restic wrapper to support streaming/progress-style output by capturing in-flight command stdout from go-sh, adding status parsing helpers, and exposing a StatusSince API to retrieve incremental progress for a given backend/repository. It also updates wrapper copy semantics to better support concurrent execution and adjusts repository integrity handling to return stats even for certain “corrupted repo” cases.
Changes:
- Update vendored
gomodules.xyz/go-shto buffer per-command output (thread-safe buffers) and exposeCurrentOutput. - Add restic status parsing (
ResticStatus,statusSince, sanitizers) plus unit tests; exposeResticWrapper.StatusSince. - Remove
circbufdependency; deepen wrapper copy logic; tweak env setup and integrity verification behavior.
Reviewed changes
Copilot reviewed 9 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/modules.txt | Updates vendored module versions and removes circbuf entry. |
| vendor/gomodules.xyz/go-sh/sh.go | Adjusts session state to support buffered output tracking. |
| vendor/gomodules.xyz/go-sh/safe_buffer.go | Adds a mutex-protected buffer type for concurrent reads/writes. |
| vendor/gomodules.xyz/go-sh/pipe.go | Enables buffering, adds CurrentOutput, and resets buffers per run. |
| vendor/github.com/armon/circbuf/README.md | Removes vendored circbuf (dependency dropped). |
| vendor/github.com/armon/circbuf/LICENSE | Removes vendored circbuf (dependency dropped). |
| vendor/github.com/armon/circbuf/circbuf.go | Removes vendored circbuf (dependency dropped). |
| vendor/github.com/armon/circbuf/.gitignore | Removes vendored circbuf (dependency dropped). |
| setup.go | Changes progress FPS and backend config resolution behavior. |
| restic_test.go | Adds tests for corrupted repository integrity stats + deep copy behavior. |
| output.go | Adds ResticStatus and incremental status parsing helpers. |
| output_test.go | Adds unit tests for statusSince cursor behavior. |
| go.sum | Updates sums for dependency changes. |
| go.mod | Drops circbuf, bumps go-sh, and adds a commented local replace line. |
| config.go | Implements deep-copy logic for wrapper/config/backend envs. |
| commands.go | Changes execution to reuse w.sh, adds --json for restore, and adds StatusSince. |
| backup.go | Adds BackupOptions callbacks/override wrapper usage; adjusts integrity verification behavior. |
| agents.md | Adds repository “Agents Guide” documentation (currently has a mismatch with code). |
Comments suppressed due to low confidence (1)
go.mod:59
- The commented
replacewith an absolute developer path should not be committed; it’s easy to accidentally uncomment during local work and it also leaks a machine-specific path into the module metadata.
//replace gomodules.xyz/go-sh => /home/anisur/go/src/gomodules/go-sh
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
| data = sanitizeFromEnd(data) | ||
| if data == nil { | ||
| klog.Infoln("status can not sanitize from end, data is not valid, ignoring it...") | ||
| klog.V(5).Infoln("status cannot be sanitized from end, data is not valid, ignoring it...") |
There was a problem hiding this comment.
We intentionally keep the logger without verbose because in our case the backup pod will run only once. There's no way to set verbose, and in one shot we don't want to lose any debug info.
| w.sh.Stderr = io.MultiWriter(os.Stderr, &errBuff) | ||
| if w.Config.Timeout != nil { | ||
| w.sh.SetTimeout(w.Config.Timeout.Duration) | ||
| } else { |
There was a problem hiding this comment.
No need, we handled the timeout with nil check.
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Anisur Rahman <anisur@appscode.com>
Signed-off-by: Tamal Saha <tamal@appscode.com>
Signed-off-by: Anisur Rahman anisur@appscode.com