feat(configuration): decompose v3 database configuration - #2084
Merged
josecelano merged 5 commits intoAug 24, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Decomposes v3 database configuration into explicit SQLite, MySQL, and PostgreSQL variants with validation, secret handling, and migration documentation.
Changes:
- Adds driver-specific database configuration and connection details.
- Adds port defaults, password validation, and redacted persistence.
- Updates migration guidance and implementation tracking.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Summary | Review status |
|---|---|---|
packages/configuration/src/v3_0_0/mod.rs |
Integrates v3 persistence and configuration tests. | No final comments |
packages/configuration/src/v3_0_0/database.rs |
Defines database variants, validation, and serialization. | Requires changes: critical load-path default merging issue; moderate SQLite path accessibility issue |
docs/issues/open/1978-configuration-overhaul-epic/configuration-v2-to-v3-migration.md |
Documents database migration examples. | No final comments |
docs/issues/open/1490-1978-decompose-database-configuration.md |
Records implementation and verification status. | No final comments |
Suppressed comments (2)
packages/configuration/src/v3_0_0/database.rs:256
- This assertion exposes the password solely for incidental test inspection. The repository's secret-handling contract requires avoiding
expose_secret()in tests when the value can be checked through the redactedSecretString/containing value; mirror the MySQL test above and compare the complete PostgreSQL value instead.
assert_eq!(connection.password.expose_secret(), "db_password");
packages/configuration/src/v3_0_0/database.rs:338
- The new security-sensitive
Debugpath is not covered here: this test only verifies custom JSON serialization. Existing token tests assert the exactSecretBox<str>([REDACTED])form and absence of the unique secret; add the same assertion forformat!("{database:?}")so a future field/derive change cannot reintroduce a database-password leak.
let serialized = serde_json::to_string(&database).expect("database configuration should serialize");
// Assert
assert!(serialized.contains("***"));
assert!(!serialized.contains(password));
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
ACK 19a2b9b |
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
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
Decompose the v3 database configuration into explicit SQLite, MySQL, and PostgreSQL variants.
pathfield withDatabase::{Sqlite3, MySQL, PostgreSQL}andConnectionInfo.SecretString, redact generic serialization, and retain an authorized persistence path.Scope boundary
Active runtime consumers, default v2 configuration files, examples, benchmarks, and E2E configuration continue to use v2 aliases. Their migration to explicit v3 imports and runtime database setup is intentionally deferred to #1980.
Verification
cargo test -p torrust-tracker-configurationcargo test --workspacelinter allFixes #1490