Skip to content

test(parquet): green RFC0013.1 + .7 via a testcontainers + LocalStack CI lane - #238

Merged
jensholdgaard merged 4 commits into
mainfrom
rfc0013-green-s3-integration-lane
Jun 16, 2026
Merged

test(parquet): green RFC0013.1 + .7 via a testcontainers + LocalStack CI lane#238
jensholdgaard merged 4 commits into
mainfrom
rfc0013-green-s3-integration-lane

Conversation

@jensholdgaard

Copy link
Copy Markdown
Owner

RFC 0013 green — stands up the S3 integration lane (RFC 0013 §6's original testcontainers plan), greening .1 and .7.

What

  • RFC0013.1 (S3 round-trip == local): encode a 300-row batch → put/get through Store::s3 (LocalStack) and the local backend → assert bytes match and decode back to identical records.
  • RFC0013.7 (endpoint override): put/get/delete through a store configured purely via S3Config::endpoint behaves like AWS S3.
  • Both spin LocalStack (S3-compatible, Apache-2.0) via testcontainers and create the bucket with the image's own awslocal (no S3-client dep). object_store's default path-style addressing already suits LocalStack — Store::s3 unchanged.

How it runs (CI-only)

The tests are #[ignore]d — testcontainers needs a Docker-API runtime (GitHub Actions has one; local containerd does not). A new required CI job s3-integration (added to ci-success's needs) runs them by name via --ignored, with AWS_* env feeding object_store's credential chain (LocalStack accepts any creds). The default cargo test --all-features compiles them and skips them.

⚠️ These S3 ITs are CI-verified-only — I can't run testcontainers on local containerd, so they're validated through the s3-integration Actions job on this PR (watch that check).

Supporting changes

  • S3Config builder (new + with_endpoint/with_region/with_prefix): S3Config is #[non_exhaustive] but had no constructor, so external crates (this test, and ourios-server later) couldn't build one — an API gap from feat(parquet): implement the S3 / S3-compatible Store backend (rfc0013 green) #235. Internal test updated to use it.
  • time 0.3.45 → 0.3.47 (RUSTSEC-2026-0009, RFC 2822 DoS), pulled transitively by testcontainers. Dev-only, so the MSRV 1.85 library claim is unaffected; cargo deny check green.

Remaining

.3/.4 (manifest CAS) build on this same lane next; .6 (WAL-stays-local) needs the server wiring the backend; .8 covered by colocated reader tests.

🤖 Generated with Claude Code

… CI lane

Stand up the S3 integration lane the RFC 0013 §6 testing strategy specified:

- RFC0013.1 (S3 round-trip == local): encode a 300-row batch, put/get it
  through `Store::s3` (LocalStack) and the local backend, and assert the bytes
  match and decode back to the same records.
- RFC0013.7 (endpoint override): a put/get/delete through a store configured
  purely via `S3Config::endpoint` behaves like AWS S3.

Both spin LocalStack (S3-compatible, Apache-2.0) via testcontainers and create
the bucket with the image's own `awslocal` (no S3-client dep). They are
`#[ignore]`d — testcontainers needs a Docker-API runtime, which GitHub Actions
has but local containerd does not — and a new CI job `s3-integration`
(required, in `ci-success`'s `needs`) runs them by name via `--ignored`, with
`AWS_*` env feeding object_store's credential chain. object_store's default
path-style addressing already suits LocalStack, so `Store::s3` is unchanged.

Supporting:
- Add the `S3Config::new` + `with_endpoint`/`with_region`/`with_prefix` builder
  — `S3Config` is `#[non_exhaustive]`, so external crates (this test, and the
  server later) could not construct one before. Internal test updated to use it.
- Bump dev-only `time` 0.3.45 → 0.3.47 (RUSTSEC-2026-0009, RFC 2822 DoS) pulled
  transitively by testcontainers; cargo-deny green. It's a dev dep, so the MSRV
  1.85 library claim is unaffected.

These S3 ITs are CI-verified-only (can't run on local containerd); validated via
the `s3-integration` Actions job.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 16, 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 10 minutes and 3 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: aa70b464-b167-4a30-9b01-94dc09b88831

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc1223 and 57ae470.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • crates/ourios-parquet/Cargo.toml
  • crates/ourios-parquet/src/store.rs
  • crates/ourios-parquet/tests/rfc0013_object_store.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0013-green-s3-integration-lane

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 a CI-only S3 integration lane to green RFC 0013 scenarios using LocalStack via testcontainers, and closes an external-construction gap in S3Config by introducing a public builder-style constructor.

Changes:

  • Implement RFC0013.1 and RFC0013.7 as #[ignore]d LocalStack-backed integration tests (run in CI via --ignored).
  • Add S3Config::new + with_endpoint/with_region/with_prefix builders and update the existing Store::s3 construction test.
  • Add a required s3-integration GitHub Actions job and the testcontainers dev-dependency (plus lockfile updates).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/ourios-parquet/tests/rfc0013_object_store.rs Adds LocalStack/testcontainers-based ignored integration tests for RFC0013.1 and RFC0013.7.
crates/ourios-parquet/src/store.rs Introduces S3Config builder-style constructor APIs and updates unit tests to use them.
crates/ourios-parquet/Cargo.toml Adds testcontainers-modules dev-dependency for LocalStack-based integration testing.
Cargo.lock Updates lockfile for new testcontainers dependency and related transitive updates (incl. time).
.github/workflows/ci.yml Adds required s3-integration job to run ignored S3 integration tests in CI.

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

Comment thread .github/workflows/ci.yml

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

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Comment thread crates/ourios-parquet/Cargo.toml Outdated
Comment thread crates/ourios-parquet/tests/rfc0013_object_store.rs Outdated

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

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Comment thread crates/ourios-parquet/src/store.rs Outdated

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

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

@jensholdgaard
jensholdgaard merged commit b33bff1 into main Jun 16, 2026
14 checks passed
@jensholdgaard
jensholdgaard deleted the rfc0013-green-s3-integration-lane branch June 16, 2026 11:53
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