Conversation
WalkthroughIntroduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)crates/*/{src,tests,benches,examples}/**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
crates/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-10-06T11:13:29.956ZApplied to files:
📚 Learning: 2025-06-04T10:21:01.388ZApplied to files:
📚 Learning: 2025-10-25T21:06:38.487ZApplied to files:
📚 Learning: 2025-05-27T06:16:12.195ZApplied to files:
📚 Learning: 2025-05-20T10:20:08.206ZApplied to files:
📚 Learning: 2025-06-17T16:21:24.384ZApplied to files:
🧬 Code graph analysis (1)crates/common/src/local_db/pipeline/adapters/apply.rs (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| db.execute_batch(&stmts).await.map_err(LocalDbError::from) | ||
| } | ||
|
|
||
| // export_dump: default no-op via trait default |
There was a problem hiding this comment.
this was supposed to be a function with no-op implementation for client side apply pipeline but i decided to only add this to the producer side pipeline to create dumps out of each orderbook db. this can be removed as this is not a shared function anymore
| Ok(batch.ensure_transaction()) | ||
| } | ||
|
|
||
| async fn persist<DB>(&self, db: &DB, batch: &SqlStatementBatch) -> Result<(), LocalDbError> |
There was a problem hiding this comment.
given this doesn't do anything with self, should this really be here?
There was a problem hiding this comment.
this allows us to use the persist function as pipeline.persist(). with that it's much easier to mock the implementation and write tests
There was a problem hiding this comment.
Why can't this be a generic that takes a phantom type if all it needs is type level info?
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 349 SIZE=L 🧠 Learnings used |
Motivation
See issues:
sync_database_with_servicesinto Testable Stages #2233Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Chores