Skip to content

fix: postgres-ownership#73

Merged
RambokDev merged 5 commits into
mainfrom
fix/postgres-ownership
Jun 29, 2026
Merged

fix: postgres-ownership#73
RambokDev merged 5 commits into
mainfrom
fix/postgres-ownership

Conversation

@RambokDev

@RambokDev RambokDev commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added support for per-database options, including an option to preserve database ownership during restore.
    • Added a second PostgreSQL test database environment for additional testing and validation.
  • Bug Fixes

    • Restores now safely clear existing connections before starting, helping avoid restore failures caused by active sessions.
    • Configuration loading now handles missing or invalid option values more predictably.
  • Tests

    • Expanded automated coverage for database options and restore behavior across supported database types.

charles-gauthereau added 5 commits June 29, 2026 17:10
Add `options: HashMap<String, serde_json::Value>` to DatabaseConfig and
`options: Option<HashMap<...>>` to InputDatabaseConfig so callers can
pass arbitrary per-database options (e.g. keep_ownership for pg_dumpall).
Update all DatabaseConfig struct literals in test files and add three
new config-service unit tests covering present, absent, and non-bool values.
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 115c9050-3519-4a17-acfb-a2f5a6b45ddd

📥 Commits

Reviewing files that changed from the base of the PR and between 39a77b1 and 34f353b.

📒 Files selected for processing (17)
  • databases.json
  • docker-compose.databases.yml
  • src/domain/postgres/cluster/restore.rs
  • src/domain/postgres/connection.rs
  • src/domain/postgres/restore.rs
  • src/services/config.rs
  • src/tests/domain/cluster/database.rs
  • src/tests/domain/cluster/mod.rs
  • src/tests/domain/firebird.rs
  • src/tests/domain/mariadb.rs
  • src/tests/domain/mongodb.rs
  • src/tests/domain/mssql.rs
  • src/tests/domain/mysql.rs
  • src/tests/domain/postgres.rs
  • src/tests/domain/redis.rs
  • src/tests/domain/valkey.rs
  • src/tests/services/config_tests.rs

📝 Walkthrough

Walkthrough

Adds a generic options: HashMap<String, serde_json::Value> field to DatabaseConfig, propagated from JSON config. Uses it to implement a keep_ownership flag that conditionally omits --no-owner/--no-privileges from pg_restore. Adds terminate_all_connections called before cluster restore. Introduces a second PostgreSQL dev service in Docker Compose with a matching databases.json entry.

Changes

PostgreSQL options and restore enhancements

Layer / File(s) Summary
DatabaseConfig options field
src/services/config.rs, src/tests/domain/*.rs, src/tests/domain/cluster/*, src/tests/services/config_tests.rs
Adds options: HashMap<String, serde_json::Value> to DatabaseConfig (required) and InputDatabaseConfig (optional), populates it in ConfigService::load with unwrap_or_default(), updates all test config literals to set an empty map, and adds unit tests verifying keep_ownership parsing behavior.
keep_ownership flag in pg_restore
src/domain/postgres/restore.rs
Reads keep_ownership from cfg.options (default false), logs whether ownership will be preserved or stripped, and conditionally omits --no-owner/--no-privileges from pg_restore for both Fc and Fd formats.
terminate_all_connections before cluster restore
src/domain/postgres/connection.rs, src/domain/postgres/cluster/restore.rs
Adds terminate_all_connections that connects to the postgres database and runs pg_terminate_backend for all non-system databases; calls it before replaying the cluster dump, returning early on failure.
Second PostgreSQL dev service
docker-compose.databases.yml, databases.json
Adds db-postgres-2 service on port 5438 with separate credentials and volume postgres-data-2, and a corresponding databases.json entry with options.keep_ownership: true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Portabase/agent#55: Modifies src/domain/postgres/restore.rs pg_restore command construction, directly overlapping with the keep_ownership flag changes in this PR.

Poem

🐇 A second postgres hops into the fold,
With options and ownership tales to be told.
Connections are severed before the restore,
--no-owner now optional, nothing is ignored.
The config map grows with a HashMap so neat,
This rabbit's refactor is tidy and complete! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/postgres-ownership

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@RambokDev RambokDev merged commit c24c0d7 into main Jun 29, 2026
1 of 2 checks passed
@RambokDev RambokDev deleted the fix/postgres-ownership branch June 29, 2026 17:12
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.52055% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/domain/postgres/restore.rs 68.42% 6 Missing ⚠️
src/domain/postgres/cluster/restore.rs 60.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

1 participant