Read export data, fidelity and validation from one native snapshot - #21
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe SQL layer now provides verified, connection-scoped read snapshots. SAV export and validation use the same snapshot for related reads. Tests cover snapshot consistency, isolation, cleanup, failure handling, and concurrent writes. ChangesConsistent SQL read snapshots
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The export and validation paths consistently use one database snapshot and release database resources before filesystem work. No actionable merge risk was identified. Sequence Diagram(s)sequenceDiagram
participant export_sav
participant _read_snapshot
participant SQL readers
participant Catalog database
export_sav->>_read_snapshot: Open one read snapshot
_read_snapshot->>Catalog database: Begin dialect-specific transaction
export_sav->>SQL readers: Read dataset, rows, metadata, and fidelity events
SQL readers->>Catalog database: Execute snapshot-scoped queries
_read_snapshot-->>export_sav: Return consistent export data
_read_snapshot->>Catalog database: Close connection and dispose engine
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Phase 3 — consistent, database-read-only export
Keeps existing public commands, signatures and result shapes. Uses native database snapshots, not dataset copies, application locks, new configuration or dependencies.
Changes
Validation
90a8350: deterministic SQLite WAL interleaving reproduces mixed descriptor, later-loss contamination, and validation schema race (3 failures).cbde051: all three pass; full local non-service suite 385 passed, 9 skipped, 53 deselected; focused checks andgit diff --checkpass.ca08100corrects only the service test's native-state observation: PyMySQL caches an OK-packet status that SELECT EOF packets do not update. MySQL/MariaDB now record disabled autocommit honestly; the real concurrent-commit/old-and-fresh export semantics remain asserted. Reproduced the probe failure and pass on a disposable MySQL 8.4.9 instance.Boundary
Data/metadata consistency uses each engine's native snapshot guarantees. SQLite transactional schema rename is covered. This does not add writer/DDL coordination for non-MVCC server operations such as PostgreSQL TRUNCATE. Database transactions are not held during filesystem work.
Progress
Phase 2 #20 is merged. Next: phase 4 — bounded batched PHP server imports, in a separate PR.
Final review and handoff
@codex reviewcompleted for latest headca08100, no findings and confirmed 👍.Phase 3 is ready for handoff. Do not merge automatically; the overall plan is not yet complete.