Switch to AsyncPgConnection and clean up tests#103
Conversation
Reviewer's GuideThis PR transitions the Postgres backend to use diesel_async’s AsyncPgConnection, restructures migration logic into a unified helper, and cleans up repetitive test setup by introducing a shared socket/ui helper and migration runner. Sequence Diagram for Updated Postgres Migration ProcesssequenceDiagram
participant AppLogic as "Application Logic (e.g., setup_database)"
participant PgMigrations as "run_migrations (Postgres)"
participant BlkTask as "tokio::task::spawn_blocking"
participant SyncPgConn as "PgConnection (sync, temporary)"
participant PgDB as "PostgreSQL Database"
AppLogic->>+PgMigrations: run_migrations(database_url)
PgMigrations->>+BlkTask: spawn_blocking_task()
BlkTask->>+SyncPgConn: PgConnection::establish(database_url)
SyncPgConn-->>-BlkTask: sync_connection
BlkTask->>+SyncPgConn: sync_connection.run_pending_migrations(MIGRATIONS)
SyncPgConn->>PgDB: Execute SQL Migrations
DB-->>SyncPgConn: Migration Result
SyncPgConn-->>-BlkTask: Migration Result
BlkTask-->>-PgMigrations: Task Result
PgMigrations-->>-AppLogic: QueryResult
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis update refactors database migration handling to provide backend-specific logic for SQLite and PostgreSQL, including changes to function signatures, type aliases, and conditional compilation. Test utilities and documentation are updated to clarify and abstract migration application, and test code is refactored to reduce duplication via helper functions. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant DB as Database (SQLite/Postgres)
participant Diesel as diesel_async/diesel
participant OS as OS Thread/Blocking Task
App->>DB: Request to run migrations
alt PostgreSQL
App->>OS: Spawn blocking task
OS->>Diesel: Establish synchronous PgConnection
Diesel->>DB: Run migrations synchronously
OS->>App: Return migration result
else SQLite
App->>Diesel: Use SyncConnectionWrapper
Diesel->>DB: Run migrations via wrapper
Diesel->>App: Return migration result
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(1 hotspot with String Heavy Function Arguments)
(1 file with String Heavy Function Arguments)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| lib.rs | 1 rule in this hotspot | 10.00 → 9.69 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| lib.rs | 1 advisory rule | 10.00 → 9.69 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| news_categories.rs | 8.65 → 10.00 | Large Method |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main.rs (2)
95-103: Consider re-using the newapply_migrationshelper to avoid duplicated cfg-gated calls.
apply_migrationsalready hides the backend differences; wiring it into the runtime code (not only tests) would remove these duplicated#[cfg]blocks and keep the public migration API in one place.
195-199: Same duplication here – could be collapsed viaapply_migrations.Applying the helper in
setup_databasewould leave just:apply_migrations(&mut conn, database).await?;and the cfg branches disappear.
tests/news_categories.rs (1)
16-59: Add socket timeouts to prevent hanging tests.If the server fails to reply, the
read_exactcalls will block indefinitely, leaving the test stuck. Setting small read/write timeouts on theTcpStream(or wrapping the I/O instd::io::Read/Writetimeouts) makes the failure mode explicit and speeds up CI diagnostics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/supporting-both-sqlite3-and-postgresql-in-diesel.md(1 hunks)src/db.rs(7 hunks)src/main.rs(2 hunks)test-util/src/lib.rs(3 hunks)tests/news_categories.rs(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/news_categories.rs (3)
src/db.rs (3)
create_bundle(306-315)create_category(290-299)conn(411-411)src/transaction.rs (6)
encode_params(468-486)decode_params(408-443)from_bytes(89-99)new(117-130)new(292-298)new(356-363)test-util/src/lib.rs (4)
apply_migrations(207-212)handshake(177-194)port(144-146)start_with_setup(107-141)
🔇 Additional comments (2)
docs/supporting-both-sqlite3-and-postgresql-in-diesel.md (1)
193-197: Documentation note looks accurate – nothing to amend.The added clarification about spawning a blocking task for Postgres migrations is consistent with the implementation in
src/db.rs.src/db.rs (1)
67-85: Postgres migration path looks correct – no issues spotted.
spawn_blockingcorrectly propagates synchronous Diesel errors back to the async context; the returnedQueryResult<()>is forwarded as the function result.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| news_categories.rs | 8.65 → 10.00 | Large Method |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| news_categories.rs | 8.61 → 10.00 | Large Method |
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
1 participant
Summary
AsyncPgConnectionfor the Postgres backendapply_migrationshelper for testsTesting
cargo clippy -- -D warningscargo test --quietcargo clippy --no-default-features --features postgres -- -D warningscargo build --no-default-features --features postgresmarkdownlint docs/supporting-both-sqlite3-and-postgresql-in-diesel.mdnixie docs/supporting-both-sqlite3-and-postgresql-in-diesel.mdhttps://chatgpt.com/codex/tasks/task_e_684a0e7bd68c8322b29a4143e9166965
Summary by Sourcery
Switch the Postgres backend to use async connections and revamp migration routines, and streamline tests by deduplicating migration and TCP framing logic through new helpers.
New Features:
apply_migrationshelper to unify running embedded migrations across SQLite and PostgreSQLlist_categorieshelper in news category tests to centralize TCP framing and response decodingEnhancements:
diesel_async::AsyncPgConnectionrun_migrationsto accept only the database URL for PostgreSQL and spawn blocking tasks for migrationDocumentation:
diesel_asyncfor queries and migration handling differences between backendsTests:
apply_migrationsandlist_categorieshelperscfgguards around SQLite-only tests in the database moduleSummary by CodeRabbit