Conversation
* feat: add as_str/from_str to PostgresDumpFormat * feat: resolve pg_dumpall/psql binary names * feat: add include_globals field to database config * feat: add pg_dumpall/psql globals dump and apply * feat: add postgres backup bundle (manifest + build + resolve) * feat: bundle globals into postgres backup when include_globals is set * feat: replay globals before pg_restore when backup archive is a bundle * refactor: bind FD restore tempdir guard once to clear unused warnings * docs: demonstrate include_globals in sample databases.json * chore: silence test-only re-export warning in non-test builds * revert: remove include_globals feature, restore plain pg_dump/pg_restore * feat: add pg_dumpall/psql binary names and is_superuser check * feat: add postgresql-cluster db type and config parsing * feat: pg_dumpall cluster backup and psql restore * feat: route postgresql-cluster through PostgresClusterDatabase * docs: add postgresql-cluster sample to databases.json * refactor: split cluster mode into cluster/ module (backup, restore, database) * test: mirror cluster tests into src/tests/domain/cluster/
|
Warning Review limit reached
More reviews will be available in 45 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds ChangesPostgreSQL Cluster Support
Google Cloud Storage Provider
Azure doc cleanup and docker-compose update
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GoogleCloudStorageProvider
participant build_client
participant upload_with_client
participant GCS
Caller->>GoogleCloudStorageProvider: upload(result, storage)
GoogleCloudStorageProvider->>GoogleCloudStorageProvider: validate backup_file, get metadata
GoogleCloudStorageProvider->>GoogleCloudStorageProvider: build_stream(encrypt, file)
GoogleCloudStorageProvider->>build_client: build_client(cfg)
build_client-->>GoogleCloudStorageProvider: Storage client
GoogleCloudStorageProvider->>GoogleCloudStorageProvider: StreamSource::from_stream(stream, total_size)
GoogleCloudStorageProvider->>upload_with_client: upload_with_client(client, bucket, path, source, force_single_shot)
upload_with_client->>GCS: write_object (single-shot or resumable)
GCS-->>upload_with_client: success/failure
upload_with_client-->>GoogleCloudStorageProvider: Result
GoogleCloudStorageProvider-->>Caller: UploadResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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: 6
🤖 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 `@databases.json`:
- Around line 118-119: The fixture in databases.json contains a real-looking
cluster password that should not be checked into source control. Replace the
hardcoded password entry with an obvious placeholder like changeme, or switch
the fixture to read the value from test environment configuration; update the
databases.json sample data so only non-sensitive dummy credentials remain.
In `@src/domain/postgres/cluster/database.rs`:
- Around line 40-44: The backup flow in `Database::backup` currently acquires a
`FileLock` and releases it manually, which is not cancellation-safe. Replace the
manual acquire/release pair with an RAII lock guard/helper in `backup` (and the
matching restore path if present) so the lock is released automatically on drop,
even if `backup::run` is aborted or returns early.
In `@src/domain/postgres/cluster/restore.rs`:
- Line 50: The restore command is hardcoding the maintenance database to
postgres, which overrides the explicit database preserved by
ConfigService::load() for postgresql-cluster. Update the restore flow in the
restore logic to use the configured maintenance database from the loaded config
instead of forcing postgres, and make sure the argument passed to the command
builder reflects that value consistently wherever the restore command is
assembled.
- Around line 46-53: The restore command in the `Command::new(&psql)` flow is
not stopping on the first SQL error because `psql -f` can continue after
failures. Update the restore invocation in `restore.rs` to pass the
`ON_ERROR_STOP` setting via psql’s `-v` flag before `-f`, so the process exit
code correctly reflects any SQL error during restore.
In `@src/tests/domain/cluster/mod.rs`:
- Around line 22-26: Fail fast in the cluster test setup instead of falling back
to a local PostgreSQL endpoint when container resolution fails. Update the
host/port lookup in the test helper that prepares the `connection::connect()`
config so it propagates the `get_host()` and `get_host_port_ipv4()` errors
rather than defaulting to `127.0.0.1:5432`, ensuring the test only runs against
the intended container.
In `@src/tests/domain/cluster/restore.rs`:
- Around line 69-76: The negative-path test is using a non-superuser username
but still passes the superuser password, so auth fails before the intended
pre-check in cluster::restore::run. Update the test setup for weak to use the
appuser password created in the fixture, and keep the call to restore::run with
env_for(&weak) so the test reaches and asserts the superuser check rather than
authentication.
🪄 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: f9f795fb-93e5-494b-9835-9aa0fbe429f2
📒 Files selected for processing (17)
databases.jsonsrc/domain/factory.rssrc/domain/postgres/cluster/backup.rssrc/domain/postgres/cluster/database.rssrc/domain/postgres/cluster/mod.rssrc/domain/postgres/cluster/restore.rssrc/domain/postgres/connection.rssrc/domain/postgres/mod.rssrc/services/config.rssrc/tests/domain/cluster/backup.rssrc/tests/domain/cluster/database.rssrc/tests/domain/cluster/mod.rssrc/tests/domain/cluster/restore.rssrc/tests/domain/mod.rssrc/tests/domain/postgres.rssrc/tests/services/config_tests.rssrc/tests/services/mod.rs
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
* fix: refactoring * chore: add google-cloud-storage and google-cloud-auth deps * feat: add GCS provider config model * feat: add GCS credential, client, and stream-source helpers StreamSource bridges build_stream's Send-only byte stream into the SDK's StreamingSource (which send_buffered requires to be Send+Sync+'static) via a bounded mpsc channel, avoiding any change to the shared UploadStream type. * feat: implement GCS StorageProvider upload * feat: register google-cloud-storage provider in factory * test: GCS upload roundtrip against fake-gcs-server * fix: format GCS bucket as projects/_/buckets/<name> in upload_with_client write_object rejects the bare bucket id with "malformed bucket name"; the production provider passed config.bucket_name unformatted, so real uploads would always fail. Format once in upload_with_client so prod and the fake-gcs-server test share the corrected path. * style: cargo fmt GCS provider files * fix: gcs
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/tests/storage/google_cloud_storage.rs (1)
20-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin
fsouza/fake-gcs-serverto a known-good tag.Using
latestmakes this integration test non-reproducible; an upstream image change can start breaking CI with no repo change here. Please pin a specific version or digest and bump it intentionally.🤖 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/storage/google_cloud_storage.rs` around lines 20 - 23, Pin the fake GCS container image used in the google_cloud_storage test to a specific known-good tag or digest instead of using latest. Update the GenericImage::new call in the test setup so the image version is explicit and reproducible, and only change it intentionally when bumping the dependency.
🤖 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 configuration currently hardcodes a real EDGE_KEY
secret, which should not be committed. Remove the inline credential from the
compose file and update the service configuration to read EDGE_KEY from an
external, untracked environment source instead; use the affected docker-compose
service definition to locate the change and then rotate/revoke the leaked key.
In `@src/services/storage/providers/google_cloud_storage/mod.rs`:
- Around line 42-58: The upload path in google_cloud_storage::mod::upload is
forwarding fs::metadata-derived total_size as an exact size hint even though
build_stream can transform the payload when encryption is enabled. Update the
upload flow so build_stream returns the post-transformation size (or another
accurate hint) and use that value instead of the pre-encryption file size. If
the transformed length cannot be known, remove the exact SizeHint for this
stream rather than advertising a potentially wrong length.
In `@src/services/storage/providers/google_cloud_storage/models.rs`:
- Around line 3-10: The GoogleCloudStorageProviderConfig struct currently
derives Debug while storing the raw service-account private_key, which can leak
secrets in logs. Update GoogleCloudStorageProviderConfig in models.rs to avoid
exposing private_key through Debug by either removing the Debug derive or
wrapping private_key in a redacted secret type and providing a custom Debug
implementation that masks it. Ensure any Debug output for this config cannot
print the PEM contents.
---
Nitpick comments:
In `@src/tests/storage/google_cloud_storage.rs`:
- Around line 20-23: Pin the fake GCS container image used in the
google_cloud_storage test to a specific known-good tag or digest instead of
using latest. Update the GenericImage::new call in the test setup so the image
version is explicit and reproducible, and only change it intentionally when
bumping the dependency.
🪄 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: 2f2ba059-4dc6-4d4b-8d61-2ba0a2ba33fe
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomldocker-compose.ymlsrc/services/storage/mod.rssrc/services/storage/providers/azure_blob/helpers.rssrc/services/storage/providers/azure_blob/models.rssrc/services/storage/providers/google_cloud_storage/helpers.rssrc/services/storage/providers/google_cloud_storage/mod.rssrc/services/storage/providers/google_cloud_storage/models.rssrc/services/storage/providers/mod.rssrc/tests/storage/google_cloud_storage.rssrc/tests/storage/mod.rs
💤 Files with no reviewable changes (2)
- src/services/storage/providers/azure_blob/models.rs
- src/services/storage/providers/azure_blob/helpers.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.
feat: add as_str/from_str to PostgresDumpFormat
feat: resolve pg_dumpall/psql binary names
feat: add include_globals field to database config
feat: add pg_dumpall/psql globals dump and apply
feat: add postgres backup bundle (manifest + build + resolve)
feat: bundle globals into postgres backup when include_globals is set
feat: replay globals before pg_restore when backup archive is a bundle
refactor: bind FD restore tempdir guard once to clear unused warnings
docs: demonstrate include_globals in sample databases.json
chore: silence test-only re-export warning in non-test builds
revert: remove include_globals feature, restore plain pg_dump/pg_restore
feat: add pg_dumpall/psql binary names and is_superuser check
feat: add postgresql-cluster db type and config parsing
feat: pg_dumpall cluster backup and psql restore
feat: route postgresql-cluster through PostgresClusterDatabase
docs: add postgresql-cluster sample to databases.json
refactor: split cluster mode into cluster/ module (backup, restore, database)
test: mirror cluster tests into src/tests/domain/cluster/
Summary by CodeRabbit
New Features
postgresql-clusterdatabase type and default to thepostgresdatabase when not set.Bug Fixes