chore(depot-client): split sqlite vfs transports#4922
chore(depot-client): split sqlite vfs transports#4922NathanFlurry wants to merge 1 commit into05-03-feat_rivetkit_support_gateway_bypass_probesfrom
Conversation
PR 4922 Review: chore(depot-client) split sqlite vfs transportsOverviewThis PR refactors the SQLite VFS transport layer from a concrete enum-based design to a trait-based design. The key motivation is removing the rivet-envoy-client dependency from depot-client, which unblocks WASM builds. The three concrete transports become:
Strengths
Issues1. Potential unused-variable warnings in release builds Variables only used inside cfg(debug_assertions) blocks will be unused in release builds:
These will warn (or error under -W warnings) in release/clippy passes. Fix with an _ = discard, allow(unused_variables), or _-prefix on the binding. 2. sqlite_error_reason duplicated across modules sqlite_error_reason now exists in both depot-client/src/transport.rs (private) and tests/inline/vfs_support.rs. Consider a pub(crate) utility in a shared location. 3. seed_page_as_cold_ref poison byte ordering Dirty pages prioritize snapshot.pages over fetched_by_pgno. If the poisoned pgno is already in snapshot.pages, the fetched_by_pgno.entry(pgno).or_insert(bytes) inserts the poison bytes but the snapshot copy takes priority -- poison bytes are silently discarded. In practice strict_direct_reopen_ignores_poisoned_mirror_and_reads_depot still passes because DirectDepotTransport never reads mirrors, but the contract of poison_mirror_page is now fragile. A comment clarifying this behavior would help. 4. DirectDepotTransport.commit no longer seeds the mirror The old non-strict Direct transport called apply_commit after depot commit to update the in-memory mirror. DirectDepotTransport skips this entirely. The new DirectMirrorTransport covers that path, but the divergence should be documented in vfs_support.rs. 5. register_with_transport (test-only) called without initial page fetch for DirectDepotTransport Tests like direct_engine_marks_vfs_dead_after_transport_errors call the cfg(test)-gated register_with_transport without fetching an initial page, differing from production. Fine for error-injection tests but a comment noting the omission would help. Minor
VerdictThe core architecture change is correct and the test adaptations are solid. Main actionable items before merge:
The WASM unblocking and transport trait introduction are the right direction. |
f1c981f to
a7e3f91
Compare
a7e3f91 to
3df0cfa
Compare
3df0cfa to
2b8b055
Compare
2b8b055 to
48fb65d
Compare
5425291 to
7143a97
Compare
48fb65d to
48cc3bb
Compare
48cc3bb to
e650129
Compare
7143a97 to
c47914d
Compare
|
Landed in main via stack-merge fast-forward push. Commits are in main; closing to match. |

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: