Skip to content

A restore with --force replaces the data directory, so an existing data dir is overwritten instead of refused after the database is already restored - #1444

Closed
rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:cli-restore-force
Closed

rudycelekli wants to merge 1 commit into
deeplethe:devfrom
rudycelekli:cli-restore-force

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Why

utopia restore --force is accepted and printed in the plan, then ignored. apply_restore refuses a non-empty target data directory with "pass --force to overwrite" whether or not the flag was passed. That check runs after pg_restore --clean, so the database has already been replaced and the files are left as they were. The flag cannot do the one thing its error message tells the operator to do.

What changes

run_restore passes args.force into apply_restore. A non-empty target is still refused without the flag, and the old files stay. With the flag, the target directory is removed before the archive's data directory is copied in, so the result is the archive's files and not a mix of both.

How it was checked

  • A new test, force_replaces_a_non_empty_data_dir, builds an archive with data/new.txt, points --target-data-dir at a directory that already holds old.txt, and uses /usr/bin/true as pg_restore. Without --force the restore is refused and old.txt remains. With --force, new.txt is the archive's bytes and old.txt is gone.
  • With the fix reverted the test fails: --force should replace the data dir: target data dir … is non-empty; pass --force to overwrite.
  • cargo test -p utopia-cli --bin utopia: 16 passed.
  • cargo fmt -p utopia-cli --check and cargo clippy -p utopia-cli --all-targets -- -D warnings pass.

Before review

  • Every commit is signed off (git commit -s)
  • cargo fmt --all --check, cargo clippy and cargo test pass for the touched crate

…ta dir is overwritten instead of refused after the database is already restored

Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

Thanks, and sorry this one was raced: #1449 fixed the same ordering (the data directory is checked before the database is changed) and is now merged, so I'm closing this as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants