feat: backup-restore-job-logs#60
Merged
Merged
Conversation
# Conflicts: # docker-compose.yml
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds JobLogger and threads Arc through database backup/restore run functions, backup service execution (executor, runner, compressor, uploader, result), compression utilities, API payloads/tests; enables container-based Codecov workflow and a justfile test recipe. ChangesBackup Job Logging and Service Integration
CI / Docker / Test infra
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 12
🧹 Nitpick comments (6)
src/services/backup/result.rs (1)
29-30: ⚡ Quick winConsider using debug level or a compact log format.
Pretty-printing the entire
logsvector at INFO level can produce verbose multi-line output if many log entries were collected. This adds noise to production observability streams.Suggested alternatives
Option 1: Use debug level
- info!("{:#?}",logs); + tracing::debug!("Job logs: {:#?}", logs);Option 2: Log a compact summary
- info!("{:#?}",logs); + info!("Sending {} job log entries to backup API", logs.len());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/backup/result.rs` around lines 29 - 30, The current info!("{:#?}", logs) call prints a pretty-printed multi-line dump of the entire logs vector; change it to either use debug logging (replace the info-level call with a debug-level call referencing the same variable name `logs`) or emit a compact summary at info level (log the count and a concise preview using `logs.len()` and a short formatted representation such as the first N entries or a truncated debug of each entry) so production logs are not flooded; update the statement that references `logs` in result.rs accordingly.src/services/backup/logger.rs (1)
134-137: ⚡ Quick winConsider adding a non-consuming entries getter.
The consuming
into_entriesforces callers to useArc::try_unwrap, which requires a fallback (see executor.rs:38,52). Adding a non-consuming method would simplify call sites and avoid silent data loss if the Arc refcount is unexpectedly > 1.Suggested addition
+ pub fn entries(&self) -> Vec<JobLogEntry> { + self.entries.lock().unwrap().clone() + } + pub fn into_entries(self) -> Vec<JobLogEntry> { self.entries.into_inner().unwrap() }Then executor.rs could call
logger.entries()instead of thetry_unwrap(...).unwrap_or_else(...)pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/backup/logger.rs` around lines 134 - 137, Add a non-consuming getter to complement into_entries: implement a pub fn entries(&self) -> Vec<JobLogEntry> that does not consume self but instead acquires the internal synchronization primitive (e.g., lock the Mutex/RwLock inside the Arc), clones or otherwise collects the Vec<JobLogEntry> contents and returns it; update call sites (e.g., in executor.rs) to call logger.entries() instead of attempting Arc::try_unwrap(...).unwrap_or_else(...) so callers avoid forced unwraps and silent data loss when the Arc refcount > 1.src/domain/mongodb/restore.rs (1)
30-35: ⚡ Quick winVerify the behavioral change from hard-fail to fallback is intentional.
This changes the restore behavior from failing when the source database name cannot be extracted from the archive to silently falling back to the configured
cfg.database. While this is more lenient and may handle edge cases gracefully, it could also mask issues such as:
- Corrupted or malformed archive metadata
- Bugs in the
extract_db_namelogic- Archives missing expected structure
If the fallback behavior is intentional, consider logging at
warnlevel instead ofinfoto make it more visible in production logs, since this represents a deviation from the expected happy path.💡 Suggested improvement
- let source_db = extract_db_name(&dry_output).unwrap_or_else(|| { - info!("Could not detect source database from archive, falling back to configured database: {}", cfg.database); - cfg.database.clone() - }); + let source_db = extract_db_name(&dry_output).unwrap_or_else(|| { + warn!("Could not detect source database from archive, falling back to configured database: {}", cfg.database); + cfg.database.clone() + });And add
warnto the import on line 6:-use tracing::{debug, error, info}; +use tracing::{debug, error, info, warn};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/mongodb/restore.rs` around lines 30 - 35, The change made in restore.rs shifts from hard-failing to silently falling back when extract_db_name(&dry_output) returns None; confirm this behavior is intended and, if so, make the fallback more visible by logging the fallback message with warn! instead of info! (update the import to include warn) and keep the subsequent info!("Using source database in archive: {}", source_db); line. Specifically, update the unwrap_or_else closure that currently calls info!("Could not detect source database...") to use warn! and ensure the logging macro import (e.g., warn) is added alongside info at the top of the file.src/tests/domain/mariadb.rs (1)
61-61: 💤 Low valueConsider a test helper for JobLogger instantiation.
The pattern
std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())is verbose and repeated multiple times across test files. Consider adding a test helper function to improve readability.♻️ Suggested test helper
Add to a common test utilities module:
#[cfg(test)] pub fn test_logger() -> std::sync::Arc<crate::services::backup::logger::JobLogger> { std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()) }Then use it as:
- let file_path = db.backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())).await.unwrap(); + let file_path = db.backup(backup_path, test_logger()).await.unwrap();Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/domain/mariadb.rs` at line 61, Introduce a small test helper to avoid repeating std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()): add a public fn test_logger() -> std::sync::Arc<crate::services::backup::logger::JobLogger> in a shared test utilities module (guarded with #[cfg(test)]) that returns the Arc-wrapped JobLogger, then replace occurrences in tests (e.g., the call inside the backup test that currently calls db.backup(... std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()) ...)) with test_logger() to improve readability and reduce duplication.src/domain/mysql/backup.rs (1)
59-59: 💤 Low valueConsider removing unused
_stdoutcapture.The
_stdoutvariable is captured but never used. If stdout output is not needed for logging or diagnostics, consider removing this line to simplify the code.♻️ Optional cleanup
- let _stdout = String::from_utf8_lossy(&output.stdout).to_string(); let stderr = String::from_utf8_lossy(&output.stderr).to_string();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/mysql/backup.rs` at line 59, Remove the unused stdout capture: delete the line creating `_stdout` (the `let _stdout = String::from_utf8_lossy(&output.stdout).to_string();`) in src/domain/mysql/backup.rs; if you intended to keep stdout for diagnostics instead, replace the unused variable with a direct log or use `let _ = &output.stdout;` or log via the existing logger, otherwise simply remove the line to eliminate dead code.src/tests/domain/mssql.rs (1)
93-93: ⚡ Quick winKeep the logger around and assert the new behavior.
These tests now satisfy the new signature, but they still throw the
JobLoggerinstances away immediately, so they won't catch regressions in the feature this PR adds. Please reuse a logger variable and assert that backup/compression recorded entries.Also applies to: 122-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/domain/mssql.rs` at line 93, The test currently discards the JobLogger returned into the backup call; instead create and reuse a named logger variable (e.g., let logger = Arc::new(crate::services::backup::logger::JobLogger::new())) and pass that to db.backup(..., logger.clone()).await.unwrap() so the test retains the logger instance, then assert on logger entries after calling backup and compression (check that logger recorded backup/compression messages or entry counts). Apply the same change to the other occurrences around the 122-125 block so both backup and compression tests verify the logger recorded expected entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.yml`:
- Line 22: The docker-compose file currently hardcodes the secret value under
the EDGE_KEY environment variable; remove the literal value from the EDGE_KEY
entry and load it from an external secret (e.g., an .env file or Docker
secrets). Specifically, replace the inline value for EDGE_KEY in the
docker-compose service with a reference to the environment variable name
EDGE_KEY (or a Docker secret) and add the real secret to a .env (ensure .env is
in .gitignore) or create a Docker secret and reference it in docker-compose;
also add a .env.example with a placeholder so contributors know the variable
name.
In `@justfile`:
- Around line 63-69: The `test` recipe in the justfile doesn't match CI: it must
set CARGO_TARGET_DIR=/app/target and run cargo clean before running tests so
coverage runs match CI; update the `test` target to export
CARGO_TARGET_DIR=/app/target (same as CI), run `cargo clean` inside the
agent-test container prior to `cargo test`, and ensure LLVM_PROFILE_FILE points
to /app/coverage/*.profraw so profraws are written to the same directory CI
collects.
In `@src/domain/mariadb/backup.rs`:
- Line 33: The local variable _mariadb_dump is mistakenly prefixed with an
underscore (suggesting it's unused) but is actually logged later; rename
_mariadb_dump to mariadb_dump where it's declared (the call to
select_mariadb_path(&version).join("mariadb-dump")) and update the subsequent
logging usage to reference mariadb_dump so the variable name matches and follows
Rust conventions.
In `@src/domain/mongodb/backup.rs`:
- Line 40: Replace the tracing::error! call used to report the MongoDB backup
failure with the injected logger.log("error", ...) call so it matches other log
usage in the function: instead of error!("MongoDB backup failed for {}: {}",
cfg.name, stderr) call logger.log("error", ...) and pass a message including
cfg.name and stderr; also remove the now-unused tracing::error import. Locate
the occurrence by the error! macro and the surrounding variables (cfg.name,
stderr) in backup.rs.
In `@src/domain/mssql/backup.rs`:
- Around line 39-52: The failure branch uses labeled "stderr: {} stdout: {}"
while the success branch builds an unlabeled combined string; make them
consistent by building a labeled combined output (e.g., "stderr: {stderr}
stdout: {stdout}") and use that for logger.log_command("sqlpackage", ...) in
both paths, keeping exit_code as exit_code on failure and Some(0) on success;
update the combined variable construction (used later in logger.log_command) to
always produce the labeled format and replace the current unlabeled
concatenation in the success path so both logger.log_command and logger.log
messages use the same output format.
In `@src/domain/postgres/backup.rs`:
- Line 94: The command label currently builds cmd_label with the full connection
URL (containing the plaintext password) and passes it to logger.log_command;
change this to use a sanitized connection string instead: parse or inspect the
URL used to construct cmd_label in this module (the code creating cmd_label and
the surrounding function in backup.rs), replace or redact the password portion
(e.g., set password to "****" or remove the userinfo) and then format cmd_label
with that sanitized URL before calling logger.log_command (leave the actual
pg_dump invocation unchanged so the password is still used for execution but
never emitted to logs).
- Around line 57-76: PostgresDumpFormat::Fc is building a connection URL with
credentials and the raw stderr variable is passed into logger.log_command for
"pg_dump", risking password leakage; before calling logger.log_command (in the
Fc branch handling Command output and in the Err(e) branch), sanitize the stderr
(and any assembled connection URL) by redacting the password or replacing the
entire postgresql://... URL with a masked version (e.g. remove
user:password@host or replace with user:****`@host`) and pass the sanitized string
to logger.log_command instead of the raw stderr (or skip logging raw stderr for
pg_dump in production).
In `@src/domain/redis/backup.rs`:
- Around line 44-45: Replace uses of stdout in the JobLogger command payload
with stderr so the command tuple is (stderr, exit_code, duration) as expected;
locate the places in src/domain/redis/backup.rs where stderr and stdout are
created (variables named stderr and stdout) and where log_command or JobLogger
is invoked (the call currently passing stdout) and change that argument to
stderr, and apply the same fix for the other occurrence around the block
referenced at lines ~81-90 to ensure all command payloads use stderr for the
stderr field.
- Around line 32-34: Remove the plaintext password arg and unify backup logging:
instead of cmd.arg("-a").arg(&cfg.password) use a non-argv auth mechanism (e.g.,
set the REDISCLI_AUTH env on the Command via cmd.env("REDISCLI_AUTH",
&cfg.password) or switch to --askpass) so the secret never appears in argv; also
capture both stdout and stderr from the redis-cli run and pass a consistent
string to logger.log_command (for example, prefer stderr if non-empty else
stdout, or combine them) so jobLogs[].output semantics are consistent between
success and failure paths. Ensure the change touches the Command invocation that
uses cfg.password and the calls to logger.log_command that currently pass
Some(stdout.to_string()) or stderr to make them use the unified output value.
In `@src/domain/sqlite/backup.rs`:
- Around line 24-25: The log currently emits the absolute database path via
logger.log which can leak host filesystem details; change the logger.log call in
backup.rs (the code that constructs db_path from db_path_str and calls
logger.log) to avoid printing db_path.display(): either log only the filename
component (use db_path.file_name() or a safe fallback) or remove the path output
entirely so JobLogger does not receive absolute filesystem paths.
In `@src/domain/valkey/backup.rs`:
- Around line 44-45: The code captures both stderr and stdout but mistakenly
passes stdout to log_command; change calls to log_command (in this file, e.g.,
where you build the command metadata after running the valkey backup process) to
use stderr (the String from String::from_utf8_lossy(&output.stderr)) instead of
stdout, and make the same replacement for the second occurrence around the other
block (lines ~81-90) so the serialized metadata follows the
stderr/exit-code/duration contract.
- Around line 32-34: The current code adds the secret via argv
(cmd.arg("-a").arg(&cfg.password)), which leaks the password to process
listings; remove that argv usage and instead set the credential through a
non-argv channel: either set the VALKEYCLI_AUTH environment variable on the
Command (e.g., cmd.env("VALKEYCLI_AUTH", &cfg.password)) or invoke valkey-cli
with the interactive/askpass mode (pass "--askpass" or equivalent) and write the
password to the child process's stdin via .stdin(Some(Stdio::piped())) before
spawning. Update the code in src/domain/valkey/backup.rs where cfg.password and
cmd are used (remove cmd.arg("-a").arg(&cfg.password)) and ensure you do not log
the password anywhere.
---
Nitpick comments:
In `@src/domain/mongodb/restore.rs`:
- Around line 30-35: The change made in restore.rs shifts from hard-failing to
silently falling back when extract_db_name(&dry_output) returns None; confirm
this behavior is intended and, if so, make the fallback more visible by logging
the fallback message with warn! instead of info! (update the import to include
warn) and keep the subsequent info!("Using source database in archive: {}",
source_db); line. Specifically, update the unwrap_or_else closure that currently
calls info!("Could not detect source database...") to use warn! and ensure the
logging macro import (e.g., warn) is added alongside info at the top of the
file.
In `@src/domain/mysql/backup.rs`:
- Line 59: Remove the unused stdout capture: delete the line creating `_stdout`
(the `let _stdout = String::from_utf8_lossy(&output.stdout).to_string();`) in
src/domain/mysql/backup.rs; if you intended to keep stdout for diagnostics
instead, replace the unused variable with a direct log or use `let _ =
&output.stdout;` or log via the existing logger, otherwise simply remove the
line to eliminate dead code.
In `@src/services/backup/logger.rs`:
- Around line 134-137: Add a non-consuming getter to complement into_entries:
implement a pub fn entries(&self) -> Vec<JobLogEntry> that does not consume self
but instead acquires the internal synchronization primitive (e.g., lock the
Mutex/RwLock inside the Arc), clones or otherwise collects the Vec<JobLogEntry>
contents and returns it; update call sites (e.g., in executor.rs) to call
logger.entries() instead of attempting Arc::try_unwrap(...).unwrap_or_else(...)
so callers avoid forced unwraps and silent data loss when the Arc refcount > 1.
In `@src/services/backup/result.rs`:
- Around line 29-30: The current info!("{:#?}", logs) call prints a
pretty-printed multi-line dump of the entire logs vector; change it to either
use debug logging (replace the info-level call with a debug-level call
referencing the same variable name `logs`) or emit a compact summary at info
level (log the count and a concise preview using `logs.len()` and a short
formatted representation such as the first N entries or a truncated debug of
each entry) so production logs are not flooded; update the statement that
references `logs` in result.rs accordingly.
In `@src/tests/domain/mariadb.rs`:
- Line 61: Introduce a small test helper to avoid repeating
std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()): add a
public fn test_logger() ->
std::sync::Arc<crate::services::backup::logger::JobLogger> in a shared test
utilities module (guarded with #[cfg(test)]) that returns the Arc-wrapped
JobLogger, then replace occurrences in tests (e.g., the call inside the backup
test that currently calls db.backup(...
std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()) ...))
with test_logger() to improve readability and reduce duplication.
In `@src/tests/domain/mssql.rs`:
- Line 93: The test currently discards the JobLogger returned into the backup
call; instead create and reuse a named logger variable (e.g., let logger =
Arc::new(crate::services::backup::logger::JobLogger::new())) and pass that to
db.backup(..., logger.clone()).await.unwrap() so the test retains the logger
instance, then assert on logger entries after calling backup and compression
(check that logger recorded backup/compression messages or entry counts). Apply
the same change to the other occurrences around the 122-125 block so both backup
and compression tests verify the logger recorded expected entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48896b4d-d70d-4f15-a4b6-a61a1c4269c6
📒 Files selected for processing (42)
.github/workflows/codecov.ymldocker-compose.test.ymldocker-compose.ymljustfilesrc/domain/factory.rssrc/domain/firebird/backup.rssrc/domain/firebird/database.rssrc/domain/mariadb/backup.rssrc/domain/mariadb/database.rssrc/domain/mongodb/backup.rssrc/domain/mongodb/database.rssrc/domain/mongodb/restore.rssrc/domain/mssql/backup.rssrc/domain/mssql/database.rssrc/domain/mysql/backup.rssrc/domain/mysql/database.rssrc/domain/postgres/backup.rssrc/domain/postgres/database.rssrc/domain/redis/backup.rssrc/domain/redis/database.rssrc/domain/sqlite/backup.rssrc/domain/sqlite/database.rssrc/domain/valkey/backup.rssrc/domain/valkey/database.rssrc/services/api/endpoints/agent/backup/mod.rssrc/services/backup/compressor.rssrc/services/backup/executor.rssrc/services/backup/logger.rssrc/services/backup/mod.rssrc/services/backup/result.rssrc/services/backup/runner.rssrc/services/backup/uploader.rssrc/tests/domain/firebird.rssrc/tests/domain/mariadb.rssrc/tests/domain/mongodb.rssrc/tests/domain/mssql.rssrc/tests/domain/mysql.rssrc/tests/domain/postgres.rssrc/tests/domain/redis.rssrc/tests/domain/valkey.rssrc/tests/utils/compress_tests.rssrc/utils/compress.rs
💤 Files with no reviewable changes (1)
- .github/workflows/codecov.yml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/codecov.yml (4)
46-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail the job when no
.profrawfiles are generated.This step only prints a count and always exits successfully. If coverage instrumentation stops writing profiles, the workflow still continues into
grcovand can upload an empty report.Suggested fix
- name: Verify profraw files exist run: | - docker compose -f docker-compose.test.yml exec \ - -e LLVM_PROFILE_FILE \ - agent-test find /app/coverage -type f -name "*.profraw" | wc -l || true + docker compose -f docker-compose.test.yml exec -T \ + -e LLVM_PROFILE_FILE \ + agent-test sh -c 'find /app/coverage -type f -name "*.profraw" -print -quit | grep -q .'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml around lines 46 - 50, The "Verify profraw files exist" step currently just prints a count and always succeeds; change it to run the docker compose command (exec as agent-test) but capture the found count (find /app/coverage -type f -name "*.profraw") into a variable and if the count is zero print a clear error and exit non‑zero so the job fails before grcov/upload; ensure the command uses the same docker compose invocation and environment (LLVM_PROFILE_FILE) as in the diff so it correctly checks the agent-test container output.
16-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an explicit minimal
GITHUB_TOKENpermission block.This workflow currently inherits repository defaults. That gives
actions/checkoutand the Codecov step more scope than they need when repo/org defaults are broader than read-only.Suggested fix
+permissions: + contents: read + jobs: coverage:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml around lines 16 - 18, Add an explicit minimal GITHUB_TOKEN permissions block to the workflow so it no longer inherits repo defaults: add a top-level or job-level permissions mapping that grants only what the steps need (e.g., permissions: contents: read) for the coverage job named "coverage" and ensure the existing actions/checkout and Codecov upload step rely on that token; update .github/workflows/codecov.yml to include the permissions block (scoped minimally, e.g., contents: read) adjacent to the jobs: coverage: runs-on declaration.Source: Linters/SAST tools
35-40:⚠️ Potential issue | 🟠 MajorAdd
-Ttodocker compose execcommands for GitHub Actions CI
docker compose execallocates a pseudo-TTY by default; on non-interactive GitHub runners this can fail with “the input device is not a TTY”.Suggested fix
- docker compose -f docker-compose.test.yml exec \ + docker compose -f docker-compose.test.yml exec -T \ -e CARGO_TARGET_DIR \ -e CARGO_INCREMENTAL \ -e RUSTFLAGS \ -e LLVM_PROFILE_FILE \ agent-test bash -c "Apply the same
-Tchange to the otherdocker compose ... execinvocations in this workflow (the “Verify profraw files exist” and “Generate coverage report inside container” steps).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml around lines 35 - 40, The CI uses "docker compose -f docker-compose.test.yml exec" which fails on non-interactive GitHub runners because exec allocates a TTY by default; update each exec invocation (the one shown and the other steps labeled "Verify profraw files exist" and "Generate coverage report inside container") to include the -T flag so the commands run without requesting a pseudo-TTY (i.e., change occurrences of docker compose ... exec to docker compose ... exec -T).
56-56:⚠️ Potential issue | 🟡 MinorUse the current rustup LLVM component name consistently (
llvm-tools).
.github/workflows/codecov.ymlinstallsllvm-tools-previewin the toolchain step, but later runsrustup component add llvm-tools. For stable Rust (1.67+), the component name isllvm-tools(legacyllvm-tools-previewwas renamed), so update thecomponents:entry tollvm-toolsto match the later command.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml at line 56, Update the CI workflow components entry to use the current Rust LLVM component name: replace the components value "llvm-tools-preview" with "llvm-tools" so it matches the later command `rustup component add llvm-tools` and maintains consistency for modern Rust toolchains; look for the `components:` list and the string "llvm-tools-preview" and change it to "llvm-tools".
🧹 Nitpick comments (1)
src/tests/domain/firebird.rs (1)
70-76: ⚡ Quick winReuse one
Arc<JobLogger>across the whole test flow.Creating two fresh loggers here only checks the updated signatures. It does not exercise the shared accumulation path that production uses for backup plus compression.
♻️ Suggested change
+ let logger = std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()); - let file_path = db.backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())).await.unwrap(); + let file_path = db.backup(backup_path, logger.clone()).await.unwrap(); ... - let compression = compress_to_tar_gz_large(&file_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())).await.unwrap(); + let compression = compress_to_tar_gz_large(&file_path, logger.clone()).await.unwrap();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/domain/firebird.rs` around lines 70 - 76, Test creates two separate Arc<JobLogger> instances which prevents exercising shared accumulation across backup and compression; instead instantiate a single Arc<JobLogger> once (e.g., let logger = std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())), pass that same logger into db.backup(..., logger.clone()).await.unwrap() and then reuse logger.clone() when calling compress_to_tar_gz_large(&file_path, logger.clone()).await.unwrap(); keep the rest of the flow (sleep, assert file_path.is_file()) unchanged so backup and compression share the same JobLogger instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/codecov.yml:
- Around line 46-50: The "Verify profraw files exist" step currently just prints
a count and always succeeds; change it to run the docker compose command (exec
as agent-test) but capture the found count (find /app/coverage -type f -name
"*.profraw") into a variable and if the count is zero print a clear error and
exit non‑zero so the job fails before grcov/upload; ensure the command uses the
same docker compose invocation and environment (LLVM_PROFILE_FILE) as in the
diff so it correctly checks the agent-test container output.
- Around line 16-18: Add an explicit minimal GITHUB_TOKEN permissions block to
the workflow so it no longer inherits repo defaults: add a top-level or
job-level permissions mapping that grants only what the steps need (e.g.,
permissions: contents: read) for the coverage job named "coverage" and ensure
the existing actions/checkout and Codecov upload step rely on that token; update
.github/workflows/codecov.yml to include the permissions block (scoped
minimally, e.g., contents: read) adjacent to the jobs: coverage: runs-on
declaration.
- Around line 35-40: The CI uses "docker compose -f docker-compose.test.yml
exec" which fails on non-interactive GitHub runners because exec allocates a TTY
by default; update each exec invocation (the one shown and the other steps
labeled "Verify profraw files exist" and "Generate coverage report inside
container") to include the -T flag so the commands run without requesting a
pseudo-TTY (i.e., change occurrences of docker compose ... exec to docker
compose ... exec -T).
- Line 56: Update the CI workflow components entry to use the current Rust LLVM
component name: replace the components value "llvm-tools-preview" with
"llvm-tools" so it matches the later command `rustup component add llvm-tools`
and maintains consistency for modern Rust toolchains; look for the `components:`
list and the string "llvm-tools-preview" and change it to "llvm-tools".
---
Nitpick comments:
In `@src/tests/domain/firebird.rs`:
- Around line 70-76: Test creates two separate Arc<JobLogger> instances which
prevents exercising shared accumulation across backup and compression; instead
instantiate a single Arc<JobLogger> once (e.g., let logger =
std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())), pass
that same logger into db.backup(..., logger.clone()).await.unwrap() and then
reuse logger.clone() when calling compress_to_tar_gz_large(&file_path,
logger.clone()).await.unwrap(); keep the rest of the flow (sleep, assert
file_path.is_file()) unchanged so backup and compression share the same
JobLogger instance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a7b6e1a7-4281-4290-b09a-39be0b84df27
📒 Files selected for processing (4)
.github/workflows/codecov.ymljustfilesrc/services/api/endpoints/agent/backup/mod.rssrc/tests/domain/firebird.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- justfile
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/domain/factory.rs (1)
11-22: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove
JobLoggerout ofservices::backupbefore widening the domain trait.
Databaseis a domain-layer contract, but it now depends oncrate::services::backup::logger::JobLogger. That pulls a service-layer, backup-namespaced type into every database implementation and the restore path as well, which inverts the dependency direction and makes this shared contract harder to evolve cleanly. A neutral module for job logging would keepsrc/domain/*independent ofsrc/services/backup/*.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/factory.rs` around lines 11 - 22, The Database domain trait currently depends on crate::services::backup::logger::JobLogger; extract a neutral logging type into the domain (e.g., create src/domain/logger.rs with a JobLogger trait or struct) and update the Database trait signatures to use that domain JobLogger instead of services::backup::logger::JobLogger (affecting fn backup(&self, backup_dir: &Path, logger: Arc<JobLogger>) and fn restore(&self, restore_file: &Path, logger: Arc<JobLogger>)). Adjust the use/import at the top of factory.rs and then update all service implementations and callers to import and pass the new domain JobLogger (or Arc<dyn JobLogger>) so the domain no longer depends on services::backup.
🧹 Nitpick comments (8)
.github/workflows/codecov.yml (3)
21-21: ⚡ Quick winAdd
persist-credentials: falseto the checkout action.For defense-in-depth, consider adding
persist-credentials: falseto prevent the GitHub token from persisting in the workspace after checkout. This reduces the attack surface if subsequent steps are compromised.Additionally, consider pinning the action to a specific commit SHA instead of
@v4for supply chain security.🔒 Recommended security hardening
- - uses: actions/checkout@v4 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml at line 21, Update the GitHub Actions checkout step (the actions/checkout usage in the workflow) to include persist-credentials: false so the GITHUB_TOKEN is not left in the workspace after checkout; while editing the same checkout step you should also consider replacing the `@v4` tag with a specific commit SHA (pin the action) to improve supply-chain security.Source: Linters/SAST tools
77-86: ⚡ Quick winConsider pinning the Codecov action to a specific commit SHA.
Using
@v5instead of a pinned SHA hash exposes the workflow to potential supply chain attacks if the action is compromised. For critical CI workflows, pinning to a specific commit SHA is a security best practice.🔒 Example of pinning to SHA
- - name: Upload coverage to Codecov - uses: codecov/codecov-action@v5 + - name: Upload coverage to Codecov + uses: codecov/codecov-action@5ecb98a3c6b747ed38dc09f787459979aebb39be # v5.0.4You can find the SHA for a specific version on the Codecov action releases page.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml around lines 77 - 86, The workflow uses the Codecov action referenced as codecov/codecov-action@v5 which is unpinned; replace the floating tag with a specific commit SHA for that release (the SHA for the desired v5 release) to prevent supply-chain risks. Update the action reference in the GitHub Actions step that currently uses codecov/codecov-action@v5 to the full pinned ref (e.g., codecov/codecov-action@<COMMIT_SHA>) and keep the existing inputs (files, flags, name, verbose, fail_ci_if_error, token, use_pypi) unchanged so the step behavior remains the same. Ensure you pick the SHA from the official Codecov action releases page and commit the updated reference.Source: Linters/SAST tools
29-41: Clarify cleanup choice for/app/targetin codecov workflow
CARGO_TARGET_DIRis set to/app/targetandCARGO_INCREMENTALis0, and the workflow doesn’t useactions/cache(onlycargo-registry/cargo-gitvolumes). Sorm -rf /app/target/*mainly forces a clean rebuild for coverage and isn’t meaningfully preserving build cache anyway.For consistency/maintainability, consider replacing the target cleanup with
cargo clean(and keep the explicit cleanup for/app/coverage/*, sincecargo cleanwon’t touch coverage).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codecov.yml around lines 29 - 41, Replace the explicit removal of /app/target/* in the "Run tests inside container" step with a call to cargo clean to perform target cleanup in a cargo-aware way; keep the explicit rm -rf /app/coverage/* to clear coverage artifacts, and ensure CARGO_TARGET_DIR and CARGO_INCREMENTAL environment variables remain unchanged so the container runs: cargo clean && rm -rf /app/coverage/* && cargo test --verbose && sync (update the bash -c string in that step accordingly).src/tests/domain/firebird.rs (1)
70-70: ⚡ Quick winConsider adding imports to simplify test code.
The verbose
std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())could be simplified by adding imports at the top of the file:use std::sync::Arc; use crate::services::backup::logger::JobLogger;Then the calls become:
Arc::new(JobLogger::new())This would improve readability across lines 70, 76, and 98.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/domain/firebird.rs` at line 70, Add local imports to shorten the fully-qualified types: add `use std::sync::Arc;` and `use crate::services::backup::logger::JobLogger;` at the top of the test file, then replace occurrences of `std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())` used as the argument to `db.backup(...)` (and similar calls) with `Arc::new(JobLogger::new())` to improve readability.src/domain/mongodb/restore.rs (1)
70-70: ⚡ Quick winLog actual exit code instead of hardcoded 0.
The success path (line 70) logs a hardcoded exit code of
0, but the actual exit code has already been extracted intoexit_code(line 61). For consistency with the failure path (line 65) and to ensure accurate command metrics, log the actualexit_codevalue.♻️ Proposed fix
- logger.log_command("mongorestore", if stderr.is_empty() { None } else { Some(stderr) }, Some(0), Some(duration_ms)); + logger.log_command("mongorestore", if stderr.is_empty() { None } else { Some(stderr) }, Some(exit_code), Some(duration_ms));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/mongodb/restore.rs` at line 70, The logger call in the success path uses a hardcoded exit code 0; replace that hardcoded 0 with the actual exit_code variable (already computed earlier) so logger.log_command("mongorestore", if stderr.is_empty() { None } else { Some(stderr) }, Some(exit_code), Some(duration_ms)) reports the real exit status; update the call in restore.rs near the mongorestore success branch to use exit_code consistent with the failure path.src/domain/mariadb/restore.rs (1)
49-49: ⚡ Quick winLog actual exit code instead of hardcoded 0.
Both success paths (lines 49 and 89) log a hardcoded exit code of
0vialogger.log_command(...), but the actual exit code has already been extracted intodrop_exit_code(line 41) andexit_code(line 80). Whilestatus.success()typically implies exit code 0, logging the actual extracted value ensures consistency and correctness in edge cases.♻️ Proposed fix
- logger.log_command("mariadb", if drop_stderr.is_empty() { None } else { Some(drop_stderr) }, Some(0), Some(drop_duration_ms)); + logger.log_command("mariadb", if drop_stderr.is_empty() { None } else { Some(drop_stderr) }, Some(drop_exit_code), Some(drop_duration_ms));And similarly for line 89:
- logger.log_command("mariadb", if stderr.is_empty() { None } else { Some(stderr) }, Some(0), Some(duration_ms)); + logger.log_command("mariadb", if stderr.is_empty() { None } else { Some(stderr) }, Some(exit_code), Some(duration_ms));Also applies to: 89-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/mariadb/restore.rs` at line 49, Replace the hardcoded exit code 0 passed to logger.log_command with the actual extracted exit code variables: use drop_exit_code for the first success path (where drop_stderr and drop_duration_ms are passed) and use exit_code for the second success path (where stderr and duration_ms are passed); locate the two calls to logger.log_command and substitute the literal 0 with the corresponding drop_exit_code / exit_code variables to ensure the logged exit code matches the extracted value.src/domain/postgres/restore.rs (1)
74-74: ⚡ Quick winLog actual exit code instead of hardcoded 0.
Both success paths (lines 74 and 176) log a hardcoded exit code of
0, but the actual exit code has already been extracted intoexit_code(lines 71 and 173). For consistency with the failure paths and to ensure accurate command metrics, log the actualexit_codevalue.♻️ Proposed fix
For line 74:
- logger.log_command("pg_restore", if combined.is_empty() { None } else { Some(combined) }, Some(0), Some(duration_ms)); + logger.log_command("pg_restore", if combined.is_empty() { None } else { Some(combined) }, Some(exit_code), Some(duration_ms));For line 176:
- logger.log_command("pg_restore", if combined.is_empty() { None } else { Some(combined) }, Some(0), Some(duration_ms)); + logger.log_command("pg_restore", if combined.is_empty() { None } else { Some(combined) }, Some(exit_code), Some(duration_ms));Also applies to: 176-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/postgres/restore.rs` at line 74, The success-path command logging in restore.rs uses a hardcoded exit code of 0 instead of the already computed exit_code. In both pg_restore success branches, update the logger.log_command calls to pass the actual exit_code variable as the exit code argument rather than Some(0), keeping the existing combined output and duration_ms handling unchanged so success and failure paths are consistent.src/services/restore/result.rs (1)
33-36: 💤 Low valueSimplify redundant error conversion.
The
map_err(|e| e.into())convertsApiErrortoApiError, which is a no-op. Since error logging already happens in the closure, you can simplify by removing.into()or returningedirectly.♻️ Proposed simplification
.await .map_err(|e| { error!("Failed to send restoration result: {}", e); - e.into() + e })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/restore/result.rs` around lines 33 - 36, The map_err closure is doing a redundant conversion by calling e.into() on an ApiError; update the closure used where the restoration result is sent (the .map_err(|e| { error!("Failed to send restoration result: {}", e); e.into() }) chain) to return the original error instead of converting it (e.g. change e.into() to e or simply return e), leaving the logging call intact so the error is still logged but without the no-op conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/restore/downloader.rs`:
- Line 18: The logger call in downloader.rs currently logs only "Failed to
download"; update the error logging in the download flow (where
logger.log("error", "Failed to download".to_string()) is invoked) to include the
HTTP response status by capturing response.status() (or equivalent status value)
and append it to the message or pass it as structured context so the log shows
both the failure text and the numeric/status code (e.g., "Failed to download
(status=...)" or include a status field).
In `@src/services/restore/executor.rs`:
- Around line 30-32: The current use of Arc::try_unwrap(logger) discards logs
when there are other Arc clones; change it to preserve entries by falling back
to cloning the inner JobLogger when try_unwrap fails: match on
Arc::try_unwrap(logger) and on Err(arc) call arc.as_ref().clone().into_entries()
(requires JobLogger: Clone) so logs are preserved instead of using
JobLogger::new(); update the code around logs, Arc::try_unwrap, JobLogger::new,
and into_entries accordingly.
In `@src/tests/domain/mssql.rs`:
- Line 141: The restore_config is being created with the same generated_id as
backup_config which can cause ID collisions; update the call to make_config that
constructs restore_config (the variable restore_config in
src/tests/domain/mssql.rs) to use a new unique generated_id string different
from the one used for backup_config (the make_config call that created
backup_config for "sourcedb"), ensuring each distinct DB config has its own
generated_id to avoid file-lock/path conflicts.
---
Outside diff comments:
In `@src/domain/factory.rs`:
- Around line 11-22: The Database domain trait currently depends on
crate::services::backup::logger::JobLogger; extract a neutral logging type into
the domain (e.g., create src/domain/logger.rs with a JobLogger trait or struct)
and update the Database trait signatures to use that domain JobLogger instead of
services::backup::logger::JobLogger (affecting fn backup(&self, backup_dir:
&Path, logger: Arc<JobLogger>) and fn restore(&self, restore_file: &Path,
logger: Arc<JobLogger>)). Adjust the use/import at the top of factory.rs and
then update all service implementations and callers to import and pass the new
domain JobLogger (or Arc<dyn JobLogger>) so the domain no longer depends on
services::backup.
---
Nitpick comments:
In @.github/workflows/codecov.yml:
- Line 21: Update the GitHub Actions checkout step (the actions/checkout usage
in the workflow) to include persist-credentials: false so the GITHUB_TOKEN is
not left in the workspace after checkout; while editing the same checkout step
you should also consider replacing the `@v4` tag with a specific commit SHA (pin
the action) to improve supply-chain security.
- Around line 77-86: The workflow uses the Codecov action referenced as
codecov/codecov-action@v5 which is unpinned; replace the floating tag with a
specific commit SHA for that release (the SHA for the desired v5 release) to
prevent supply-chain risks. Update the action reference in the GitHub Actions
step that currently uses codecov/codecov-action@v5 to the full pinned ref (e.g.,
codecov/codecov-action@<COMMIT_SHA>) and keep the existing inputs (files, flags,
name, verbose, fail_ci_if_error, token, use_pypi) unchanged so the step behavior
remains the same. Ensure you pick the SHA from the official Codecov action
releases page and commit the updated reference.
- Around line 29-41: Replace the explicit removal of /app/target/* in the "Run
tests inside container" step with a call to cargo clean to perform target
cleanup in a cargo-aware way; keep the explicit rm -rf /app/coverage/* to clear
coverage artifacts, and ensure CARGO_TARGET_DIR and CARGO_INCREMENTAL
environment variables remain unchanged so the container runs: cargo clean && rm
-rf /app/coverage/* && cargo test --verbose && sync (update the bash -c string
in that step accordingly).
In `@src/domain/mariadb/restore.rs`:
- Line 49: Replace the hardcoded exit code 0 passed to logger.log_command with
the actual extracted exit code variables: use drop_exit_code for the first
success path (where drop_stderr and drop_duration_ms are passed) and use
exit_code for the second success path (where stderr and duration_ms are passed);
locate the two calls to logger.log_command and substitute the literal 0 with the
corresponding drop_exit_code / exit_code variables to ensure the logged exit
code matches the extracted value.
In `@src/domain/mongodb/restore.rs`:
- Line 70: The logger call in the success path uses a hardcoded exit code 0;
replace that hardcoded 0 with the actual exit_code variable (already computed
earlier) so logger.log_command("mongorestore", if stderr.is_empty() { None }
else { Some(stderr) }, Some(exit_code), Some(duration_ms)) reports the real exit
status; update the call in restore.rs near the mongorestore success branch to
use exit_code consistent with the failure path.
In `@src/domain/postgres/restore.rs`:
- Line 74: The success-path command logging in restore.rs uses a hardcoded exit
code of 0 instead of the already computed exit_code. In both pg_restore success
branches, update the logger.log_command calls to pass the actual exit_code
variable as the exit code argument rather than Some(0), keeping the existing
combined output and duration_ms handling unchanged so success and failure paths
are consistent.
In `@src/services/restore/result.rs`:
- Around line 33-36: The map_err closure is doing a redundant conversion by
calling e.into() on an ApiError; update the closure used where the restoration
result is sent (the .map_err(|e| { error!("Failed to send restoration result:
{}", e); e.into() }) chain) to return the original error instead of converting
it (e.g. change e.into() to e or simply return e), leaving the logging call
intact so the error is still logged but without the no-op conversion.
In `@src/tests/domain/firebird.rs`:
- Line 70: Add local imports to shorten the fully-qualified types: add `use
std::sync::Arc;` and `use crate::services::backup::logger::JobLogger;` at the
top of the test file, then replace occurrences of
`std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())` used as
the argument to `db.backup(...)` (and similar calls) with
`Arc::new(JobLogger::new())` to improve readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ae29897-f3c6-47c4-ae67-c2ad5080f16a
📒 Files selected for processing (42)
.github/workflows/codecov.ymlsrc/domain/factory.rssrc/domain/firebird/backup.rssrc/domain/firebird/database.rssrc/domain/firebird/restore.rssrc/domain/mariadb/backup.rssrc/domain/mariadb/database.rssrc/domain/mariadb/restore.rssrc/domain/mongodb/backup.rssrc/domain/mongodb/database.rssrc/domain/mongodb/restore.rssrc/domain/mssql/backup.rssrc/domain/mssql/database.rssrc/domain/mssql/restore.rssrc/domain/mysql/backup.rssrc/domain/mysql/database.rssrc/domain/mysql/restore.rssrc/domain/postgres/backup.rssrc/domain/postgres/database.rssrc/domain/postgres/restore.rssrc/domain/redis/backup.rssrc/domain/redis/database.rssrc/domain/sqlite/backup.rssrc/domain/sqlite/database.rssrc/domain/sqlite/restore.rssrc/domain/valkey/backup.rssrc/domain/valkey/database.rssrc/services/api/endpoints/agent/backup/mod.rssrc/services/api/endpoints/agent/restore/mod.rssrc/services/backup/executor.rssrc/services/backup/result.rssrc/services/restore/archive.rssrc/services/restore/downloader.rssrc/services/restore/executor.rssrc/services/restore/result.rssrc/services/restore/runner.rssrc/tests/domain/firebird.rssrc/tests/domain/mariadb.rssrc/tests/domain/mongodb.rssrc/tests/domain/mssql.rssrc/tests/domain/mysql.rssrc/tests/domain/postgres.rs
🚧 Files skipped from review as they are similar to previous changes (14)
- src/services/backup/result.rs
- src/domain/redis/database.rs
- src/services/backup/executor.rs
- src/services/api/endpoints/agent/backup/mod.rs
- src/domain/valkey/database.rs
- src/domain/mssql/backup.rs
- src/domain/mariadb/backup.rs
- src/domain/firebird/backup.rs
- src/domain/sqlite/backup.rs
- src/domain/mysql/backup.rs
- src/domain/redis/backup.rs
- src/domain/mongodb/backup.rs
- src/domain/valkey/backup.rs
- src/domain/postgres/backup.rs
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
1 participant
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.
Summary by CodeRabbit
New Features
Tests
Chores