Skip to content

feat(parquet): rfc0013 green — store local I/O surface (put/get/delete) - #231

Merged
jensholdgaard merged 2 commits into
mainfrom
rfc0013-green-store-io
Jun 15, 2026
Merged

feat(parquet): rfc0013 green — store local I/O surface (put/get/delete)#231
jensholdgaard merged 2 commits into
mainfrom
rfc0013-green-store-io

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 15, 2026

Copy link
Copy Markdown
Owner

What

First green slice for RFC 0013 (object-storage backend): the Store I/O surface over object_store, exercised on the LocalFileSystem backend. Foundation that the consumer-migration slices build on; RFC stays red until all 8 §5 stubs pass.

  • Store::{put, get, delete} (async, Vec<u8>) over object_store::ObjectStoreExt, with resolve() joining a /-delimited key under the store prefix (prefix empty at this stage — scoping is a later slice, RFC0013.5).
  • A store::tests unit test: byte object round-trips through Store::local, and a delete removes it.
  • tokio dev-dep (the I/O surface is async; already in the workspace tree).

Verified locally

cargo fmt, cargo clippy -p ourios-parquet --all-targets (clean), cargo test -p ourios-parquet (the new store::tests::local_store_put_get_delete_round_trip passes; the 8 RFC0013 §5 stubs stay #[ignore]d), cargo deny check (ok — no new graph entries; the aws feature for S3 comes in a later slice).

Not in this slice (the green phasing)

This deliberately does not migrate consumers yet. Remaining green slices (each its own PR, un-ignoring stubs as they land):

  1. Migrate the manifest (RFC 0009) onto Store + conditional PUT (PutMode::Create/Update) → RFC0013.4.
  2. Migrate writer/reader onto Store → RFC0013.2 / .8 (local backend, no regression; forward-compat).
  3. Add the S3 backend (object_store aws feature) + MinIO testcontainers lane → RFC0013.1 / .3 / .5 / .7.
  4. WAL-stays-local assertion via the server wiring → RFC0013.6.

The migration strategy (thread &Store through consumers vs additive) and the single-writer lease mechanism are decided in slices 1–3 — I'll confirm the strategy before slice 2.

Invariants

Adds the seam's I/O surface; no consumer uses it yet (behaviour unchanged). Serves §3.6; WAL stays local (§3.4).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Storage backend now supports core operations for managing parquet objects: put (store), get (retrieve), and delete
  • Tests
    • Added round-trip tests verifying local storage operations work end-to-end

@jensholdgaard
jensholdgaard requested a review from Copilot June 15, 2026 17:23
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 44 minutes and 56 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 121b0760-63d5-40bb-a3f1-49f3550c914d

📥 Commits

Reviewing files that changed from the base of the PR and between cd825fa and cf2d6ec.

📒 Files selected for processing (1)
  • crates/ourios-parquet/src/store.rs
📝 Walkthrough

Walkthrough

Adds three public async methods (put, get, delete) to Store in ourios-parquet, backed by a private resolve() helper that converts a string key into an ObjectPath. Errors are mapped to StoreError::Backend. A tokio dev-dependency and a local round-trip unit test are added to verify the full lifecycle.

Changes

Store async CRUD via object_store

Layer / File(s) Summary
Store path resolver and async CRUD methods
crates/ourios-parquet/src/store.rs
Imports PutPayload, adds a private resolve() method that builds an ObjectPath from prefix + key, and implements public put, get, and delete async methods delegating to the underlying ObjectStore with StoreError::Backend mapping.
Tokio dev-dependency and local round-trip test
crates/ourios-parquet/Cargo.toml, crates/ourios-parquet/src/store.rs
Adds tokio (rt, macros) as a dev-dependency and a unit test exercising put/get/delete against a temp-dir local backend, asserting the object is absent after deletion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • jensholdgaard/ourios#230: Introduced the initial Store/local()/s3() scaffold in ourios-parquet/src/store.rs that this PR directly extends with CRUD wiring.

Poem

🐇 Hippity-hop through the object store lane,
A put and a get and a delete campaign!
Keys turn to paths with a resolve() so neat,
Round-trip tests confirm nothing's incomplete.
The bytes hop in, and the bytes hop out —
That's what async CRUD is all about! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing the Store I/O surface (put/get/delete methods) as the first green slice for RFC0013 object-storage backend.
Description check ✅ Passed The description is comprehensive and complete, covering the what/why, related RFC context, verification steps, and future scope with all required template sections adequately addressed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0013-green-store-io

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the first “green” slice of RFC 0013’s object-storage seam in ourios-parquet by introducing a minimal async Store I/O surface over object_store (exercised against the local filesystem backend), without migrating any consumers yet.

Changes:

  • Add Store::{put,get,delete} async APIs plus internal key resolution under the store prefix.
  • Add a unit test that round-trips bytes through the local backend and verifies delete behavior.
  • Add tokio as a dev-dependency to support the async test (and update Cargo.lock accordingly).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/ourios-parquet/src/store.rs Implements put/get/delete, path resolution, and adds a local backend round-trip test.
crates/ourios-parquet/Cargo.toml Adds tokio as a dev-dependency for the new async test.
Cargo.lock Records the new tokio dev-dependency in the lockfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ourios-parquet/src/store.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants