feat(commands): add backup command #9
Conversation
WalkthroughThe codebase now includes a new backup feature. It introduces a command to back up a specified directory, halt the "Tipi" service, and ensure its data is preserved using Docker and Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
Cargo.lockis excluded by:!**/*.lockCargo.tomlis excluded by:!**/*.toml
Files selected for processing (5)
- src/args.rs (1 hunks)
- src/commands/backup.rs (1 hunks)
- src/commands/mod.rs (1 hunks)
- src/main.rs (1 hunks)
- src/utils/system.rs (1 hunks)
Files skipped from review due to trivial changes (2)
- src/commands/mod.rs
- src/utils/system.rs
Additional comments: 2
src/main.rs (1)
- 34-36: The implementation for the
Backupcommand in themainfunction is straightforward and follows the existing pattern for other commands. However, ensure that thecommands::backup::run()function is designed to handle any required arguments or configurations that might be needed for the backup process.Verification successful
The
rgcommand has successfully located therunfunction within thebackup.rsfile, and the output indicates that the function does not take any arguments. This confirms that therunfunction is designed to be called without any parameters, as is done in themain.rsfile.Based on this information, the review comment suggesting to ensure that the
commands::backup::run()function is designed to handle any required arguments or configurations is not applicable, as the function signature indicates that it does not accept any arguments.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the backup command does not require additional arguments. ast-grep --lang rust --pattern $'fn run($_) { $$$ }' src/commands/backup.rsLength of output: 76
Script:
#!/bin/bash # Verify the signature of the `run` function in the `backup.rs` file. ast-grep --lang rust --pattern $'fn run($$$) { $$$ }' src/commands/backup.rsLength of output: 77
Script:
#!/bin/bash # Use ripgrep to search for the `run` function within the `backup.rs` file. rg "fn run" src/commands/backup.rsLength of output: 51
src/args.rs (1)
- 55-56: The addition of the
Backupvariant to theRuntipiMainCommandenum is consistent with the existing command structure. Ensure that the documentation for theBackupcommand is clear and provides enough information for users to understand its purpose and usage.
| pub fn run() { | ||
| let spin = spinner::new(""); | ||
| let root_folder: PathBuf = env::current_dir().expect("Unable to get current directory"); | ||
|
|
||
| spin.set_message("Creating backup directory..."); | ||
|
|
||
| if let Err(e) = fs::create_dir_all(root_folder.join("backups")) { | ||
| spin.fail(e.to_string().as_str()); | ||
| spin.finish(); | ||
| process::exit(1); | ||
| } | ||
|
|
||
| spin.succeed("Created backup directory"); | ||
|
|
||
| spin.set_message("Stopping Tipi..."); | ||
|
|
||
| let args = vec!["down", "--remove-orphans", "--rmi", "local"]; | ||
|
|
||
| process::Command::new("docker") | ||
| .arg("compose") | ||
| .args(&args) | ||
| .output() | ||
| .expect("Failed to stop containers. Cannot continue with backup"); | ||
|
|
||
| spin.succeed("Tipi successfully stopped"); | ||
|
|
||
| spin.set_message("Backing up tipi..."); | ||
|
|
||
| if let Err(e) = backup(root_folder) { | ||
| spin.fail(e.to_string().as_str()); | ||
| spin.finish(); | ||
| return; | ||
| } | ||
|
|
||
| spin.succeed("Tipi Backed Up!"); | ||
| spin.finish(); | ||
| } |
There was a problem hiding this comment.
The run function in backup.rs is responsible for creating the backup directory, stopping the "Tipi" service, and performing the backup. The error handling within this function exits the process immediately upon encountering an error, which may not be the best approach. Consider returning a Result type and allowing the caller to decide how to handle the error.
- process::exit(1);
+ return Err(e);| pub fn backup(root_folder: PathBuf) -> Result<(), Error> { | ||
| let datetime = chrono::Utc::now(); | ||
| let parent_folder = root_folder.parent().unwrap(); | ||
| let root_folder_string = root_folder.to_str().unwrap(); | ||
| let filename = format!("runtipi-backup-{}.tar.gz", datetime.format("%d-%m-%Y")); | ||
| let tar_path = format!("{}/{}", parent_folder.to_str().unwrap(), filename); | ||
| let backups_folder = format!("{}/backups/{}", root_folder_string, filename); | ||
|
|
||
| env::set_current_dir(&parent_folder)?; | ||
|
|
||
| process::Command::new("tar").args(["-czvf", &filename, &root_folder_string]).output().expect("Failed to run tar command"); | ||
| fs::rename(tar_path, backups_folder).expect("Failed to move the tar archive."); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
The backup function creates a tarball of the current directory. Ensure that the backup includes all necessary files and that no sensitive information is inadvertently included in the backup. Additionally, the use of unwrap could cause a panic if the parent() or to_str() calls fail. Replace unwrap with proper error handling.
- let parent_folder = root_folder.parent().unwrap();
- let root_folder_string = root_folder.to_str().unwrap();
+ let parent_folder = root_folder.parent().ok_or_else(|| Error::new(ErrorKind::Other, "Failed to get parent directory"))?;
+ let root_folder_string = root_folder.to_str().ok_or_else(|| Error::new(ErrorKind::Other, "Failed to convert root folder to string"))?;
meienberger
force-pushed
the
main
branch
2 times, most recently
from
04c4681
to
9fccfd2
Compare
meienberger
force-pushed
the
main
branch
2 times, most recently
from
b7c48e5
to
080f72c
Compare
Summary by CodeRabbit