Interactive restore and backup script #1226
Conversation
… choose which restore file you want to use, ask for confirmation before restoring. backup file adjusted to work with the new restore bash
WalkthroughTwo new Bash scripts introduce automated backup and restore functionality for a Riven + PostgreSQL deployment. The backup script periodically archives the database and app data with retention policies, while the restore script provides interactive recovery from available backups with confirmation prompts. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Scheduler
participant BkupScript as backup-riven.sh
participant AppContainer as App Container
participant DBContainer as DB Container
participant FileSystem as Filesystem
User->>BkupScript: Execute script
BkupScript->>BkupScript: Initialize env & timestamp
BkupScript->>AppContainer: Stop container
BkupScript->>DBContainer: Ensure running
BkupScript->>DBContainer: Wait for readiness
BkupScript->>DBContainer: Dump DB to compressed file
DBContainer-->>FileSystem: riven-db-[date].dump.gz
BkupScript->>FileSystem: Archive Riven data
FileSystem-->>FileSystem: riven-data-[date].tar.gz
BkupScript->>AppContainer: Restart container
BkupScript->>FileSystem: Delete backups older than 7 days
BkupScript-->>User: Completion notice
rect rgba(76, 175, 80, 0.1)
note over BkupScript,FileSystem: Cleanup Phase
end
sequenceDiagram
participant User as User
participant RestoreScript as restore-riven.sh
participant FileSystem as Filesystem
participant AppContainer as App Container
participant DBContainer as DB Container
User->>RestoreScript: Execute script
RestoreScript->>FileSystem: Scan for available backups
RestoreScript->>User: Present backup options
User->>RestoreScript: Select restore type & backup
RestoreScript->>User: Confirm restore (double-check)
User->>RestoreScript: Confirm
RestoreScript->>AppContainer: Stop container
alt Restore Database
RestoreScript->>DBContainer: Start container
RestoreScript->>DBContainer: Wait for readiness
RestoreScript->>DBContainer: Drop & recreate DB
RestoreScript->>DBContainer: Restore from dump
end
alt Restore App Data
RestoreScript->>FileSystem: Extract tarball
FileSystem-->>FileSystem: Extract to RIVEN_DATA
end
RestoreScript->>AppContainer: Restart container
RestoreScript-->>User: Success/status message
rect rgba(33, 150, 243, 0.1)
note over RestoreScript,DBContainer: Selective Restore
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The addition involves two new shell scripts with container orchestration and database operations. While individual components are straightforward, the combination of dependency management (PostgreSQL readiness, container lifecycle), file operations with retention policies, and error handling across both scripts requires careful verification of control flow and edge cases. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
backup-riven.sh (3)
24-29: Use_instead of unused loop variablei.The loop counter
iis only used for iteration count and timeout, not accessed in the loop body. Replace with_to satisfy shellcheck and clarify intent.-for i in {1..30}; do +for _ in {1..30}; do
7-8: Make database name and PostgreSQL password configurable.Lines 33 and 32 hardcode the database name (
riven) and password (postgres). Users with different database names or credentials cannot use this script without editing it. Add variables at the top for flexibility:RIVEN_CONTAINER="riven" RIVEN_DB_CONTAINER="riven-db" RIVEN_DATA="./riven/data" +RIVEN_DB_NAME="riven" +POSTGRES_PASSWORD="postgres"Then update the dump command:
-docker exec -e PGPASSWORD=postgres "$RIVEN_DB_CONTAINER" \ - pg_dump -U postgres -d riven -Fc > "$BACKUP_DIR/riven-db-$DATE.dump" +docker exec -e PGPASSWORD="$POSTGRES_PASSWORD" "$RIVEN_DB_CONTAINER" \ + pg_dump -U postgres -d "$RIVEN_DB_NAME" -Fc > "$BACKUP_DIR/riven-db-$DATE.dump"Also applies to: 32-33
15-16: Document the rationale for|| trueerror suppression.The script uses
|| trueto silently suppress errors when stopping/starting containers. This is reasonable if containers may already be stopped, but it also masks unexpected failures (e.g., permission denied, docker daemon unavailable). Consider adding a comment explaining this choice, or consider failing loudly on unexpected errors:+# Container may already be stopped; suppress "not found" errors -docker stop "$RIVEN_CONTAINER" || true +docker stop "$RIVEN_CONTAINER" 2>/dev/null || trueAlso applies to: 44-45
restore-riven.sh (4)
89-94: Use_instead of unused loop variablei.The loop counter
iis only used for iteration count and timeout, not accessed in the loop body. Replace with_to satisfy shellcheck and clarify intent.- for i in {1..30}; do + for _ in {1..30}; do
72-72: Document Bash 4.0+ requirement for lowercase parameter expansion.Lines 72 and 75 use the
${var,,}syntax (e.g.,${confirm,,}) to convert input to lowercase. This syntax requires Bash 4.0 or later and will fail silently on older shells. Either add a version check at the top of the script or document this requirement in a comment:+#!/bin/bash +# restore-riven.sh — interactive restore (DB only / data only / both) +# Requires: Bash 4.0+Alternatively, use a portable approach:
-[[ "${confirm,,}" == "y" ]] || { echo "Aborted."; exit 0; } +[[ "$(echo "$confirm" | tr '[:upper:]' '[:lower:]')" == "y" ]] || { echo "Aborted."; exit 0; }Also applies to: 75-75
96-107: Make database name configurable to match backup script.The restore script hardcodes the database name
riven(lines 98, 100, 105). For consistency with parameterization and user flexibility, add aRIVEN_DB_NAMEvariable near the top (as suggested in the backup script review):RIVEN_CONTAINER="riven" RIVEN_DB_CONTAINER="riven-db" +RIVEN_DB_NAME="riven" RIVEN_DATA="./riven/data"Then update the restore logic:
- docker exec -i -e PGPASSWORD=postgres "$RIVEN_DB_CONTAINER" \ - psql -U postgres -c "DROP DATABASE IF EXISTS riven;" >/dev/null - docker exec -i -e PGPASSWORD=postgres "$RIVEN_DB_CONTAINER" \ - psql -U postgres -c "CREATE DATABASE riven;" >/dev/null + docker exec -i -e PGPASSWORD=postgres "$RIVEN_DB_CONTAINER" \ + psql -U postgres -c "DROP DATABASE IF EXISTS $RIVEN_DB_NAME;" >/dev/null + docker exec -i -e PGPASSWORD=postgres "$RIVEN_DB_CONTAINER" \ + psql -U postgres -c "CREATE DATABASE $RIVEN_DB_NAME;" >/dev/nullAnd the restore command:
- pg_restore -U postgres -d riven --no-owner -F c < "$DB_DUMP" + pg_restore -U postgres -d "$RIVEN_DB_NAME" --no-owner -F c < "$DB_DUMP"
40-51: Add validation and timeout toselectloops for better UX.The
selectloops (lines 47–49 and 61–63) can hang indefinitely waiting for user input. While acceptable for an interactive script, consider adding a timeout or clearer guidance. Additionally, if a user provides invalid input repeatedly, they may need to interrupt the script. Add a counter or document this behavior:echo "Available database backups:" +PS3="Select a file (or press Ctrl+C to cancel): " select DB_DUMP in "${DB_FILES[@]}"; do [[ -n "${DB_DUMP:-}" ]] && break doneAlternatively, validate the selection more explicitly:
select DB_DUMP in "${DB_FILES[@]}"; do - [[ -n "${DB_DUMP:-}" ]] && break + if [[ -n "${DB_DUMP:-}" ]]; then + echo "✓ Selected: $(basename "$DB_DUMP")" + break + else + echo "Invalid selection. Please try again." + fi doneAlso applies to: 54-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backup-riven.sh(1 hunks)restore-riven.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
restore-riven.sh
[warning] 89-89: i appears unused. Verify use (or export if used externally).
(SC2034)
backup-riven.sh
[warning] 24-24: i appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (2)
backup-riven.sh (1)
37-42: Verify tar structure matches restore expectations.The tar command archives with
-C "$RIVEN_DATA" ., which places archive contents at the root (no parent directory). Confirm thatrestore-riven.shextracts into$RIVEN_DATAand expects this structure. If the archive should include the$RIVEN_DATAdirectory itself, usetar czf ... -C "$(dirname "$RIVEN_DATA")" "$(basename "$RIVEN_DATA")"instead.restore-riven.sh (1)
109-115: Verify tar extraction structure matches backup archive.The restore command
tar xzf "$DATA_TAR" -C "$RIVEN_DATA"assumes the backup archive (created bytar czf ... -C "$RIVEN_DATA" .) contains relative paths that should be extracted into$RIVEN_DATA. This coupling is correct if the backup script structure is maintained, but any deviation (e.g., absolute paths, different tar format) could silently extract files to unexpected locations or corrupt the directory structure.Consider adding a pre-flight check to validate the archive contents:
echo "Archive contents preview:" tar tzf "$DATA_TAR" | head -10Or add a safety check to ensure
$RIVEN_DATAis not overwritten (mkdir -p is already done on line 112, which is safe).
|
Theres already backup and restore built into the cli of Riven. It's here, |
|
Actually on second thought, maybe an external one might not be so bad.. what you think @Gaisberg ? |
|
Imho this should not be included in the repository as is, but instead expand the snapshotting and settings management within. |
|
if it's in the cli, we could do a simpler |
3 participants
Pull Request Check List
Resolves: none
Description:
updated backup and restore script, that is more interactive, lets you… choose which restore file you want to use, ask for confirmation before restoring. backup file adjusted to work with the new restore bash
Summary by CodeRabbit