Make backups transfer with servers#155
Conversation
📝 WalkthroughWalkthroughAdds backup support to server transfer: request payload now includes Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant Transfer
participant Archive
Client->>Router: POST /server/transfer { url, token, backups }
Router->>Transfer: PushArchiveToTarget(url, token, backups)
Transfer->>Transfer: set BackupUUIDs = backups
alt backups provided
Transfer->>Transfer: emit streaming message
Transfer->>Archive: StreamBackups(ctx, mp)
Archive-->>Transfer: streaming completed
else no backups
Transfer->>Transfer: log "no backups specified"
end
Transfer-->>Router: return archive response
Router-->>Client: 200 OK (transfer complete)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/transfer/archive.go (1)
38-38: Minor grammar issue in comment.The comment reads "if there at least is 1 backup" but should be "if there is at least 1 backup".
🔎 Suggested fix
- // In theory this can't happen as this function is only called if there at least is 1 backup but just to be sure + // In theory this can't happen as this function is only called if there is at least 1 backup but just to be sure
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
router/router_server_transfer.goserver/transfer/archive.goserver/transfer/source.go
🧰 Additional context used
🧬 Code graph analysis (2)
server/transfer/source.go (1)
server/transfer/transfer.go (1)
Transfer(43-60)
router/router_server_transfer.go (2)
config/config.go (2)
Token(305-308)Backups(257-279)server/installer/installer.go (1)
ServerDetails(17-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and Test (ubuntu-22.04, 1.25.1, linux, amd64)
- GitHub Check: Build and Test (ubuntu-22.04, 1.24.7, linux, amd64)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
server/transfer/archive.go (1)
37-133: LGTM! Backup streaming implementation is solid.The defensive zero-backups check is appropriate, and the implementation correctly handles:
- File discovery and filtering by UUID
- Individual backup checksums for integrity verification
- Progress tracking with detailed logging
- Proper resource cleanup (file handles closed)
router/router_server_transfer.go (2)
21-24: LGTM! Backups field correctly added.The
Backupsfield is appropriately optional (nobinding:"required"tag), allowing the panel to send an empty array when no backups should be transferred, as documented in the PR description.
84-84: LGTM! Call site updated correctly.The call to
PushArchiveToTargetnow passes the backups array as expected by the updated method signature.server/transfer/source.go (2)
17-17: LGTM! Method signature updated to support backups.The addition of the
backups []stringparameter enables backup transfer functionality. As noted in the PR description, this requires a coordinated change in the panel repository (app/Services/Servers/TransferServerService.php) to return backup UUIDs.Confirm that the corresponding panel changes have been implemented or are being coordinated to return the backup UUIDs array.
127-137: LGTM! Backup streaming logic is well-implemented.The implementation correctly:
- Stores backup UUIDs in the Transfer struct before use
- Provides user-friendly progress messages with backup counts
- Conditionally streams backups only when provided
- Logs appropriately when no backups are specified
- Properly propagates errors from the streaming operation
Changes
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.