cache-control to no-store for metadata uploads to S3

[nikhilsinhaparseable]

gated by env P_S3_CACHE_CONTROL_NO_STORE (default false)
this is as per the current behaviour

if set to true, for all non parquet uploads to S3 (gcs/azure-blob no impact)
Cache-Control header is set to true

before

{
 "status": "success",
 "name": ".stream.json",
 "lastModified": "2026-07-14T21:44:30+05:30",
 "size": 872,
 "etag": "ccc075686723f427567d10d22a358c7b",
 "type": "file",
 "metadata": {
  "Content-Type": "binary/octet-stream"
 }

after

}
box3@box3-MS-7E88:~/quest$ mc stat --json local/cachetest/teststream1/.stream/.stream.json
{
 "status": "success",
 "name": ".stream.json",
 "lastModified": "2026-07-14T22:00:31+05:30",
 "size": 878,
 "etag": "391bfbb2387ffd1673549cd83d99ce9f",
 "type": "file",
 "metadata": {
  "Cache-Control": "no-store",
  "Content-Type": "binary/octet-stream"
 }
}

Summary by CodeRabbit

• New Features

  • Added an optional setting to prevent caching of non-Parquet objects stored through S3.
  • The setting is disabled by default and can be configured through the S3 environment/CLI configuration.
  • Parquet objects remain cacheable when the setting is enabled.

• Tests

  • Added coverage for default behavior, non-Parquet cache prevention, and Parquet handling.

[coderabbitai]

Walkthrough

Adds configurable no-store cache control for non-Parquet S3 uploads, preserves Parquet caching, and wires the option through both S3 client construction paths with unit tests.

Changes

Conditional object cache control

Layer / File(s)	Summary
MetricLayer cache-control behavior src/storage/metrics_layer.rs	MetricLayer adds a configurable cache-control flag, applies no-store to regular and multipart non-Parquet uploads, and tests default, JSON, and Parquet behavior.
S3 configuration wiring src/storage/s3.rs	S3Config adds the P_S3_CACHE_CONTROL_NO_STORE option and passes it to both MetricLayer construction paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: parmesant

Poem

A rabbit hops through storage bright,
Setting cache flags just right.
JSON rests where caches close,
Parquet keeps its cached repose.
“Upload onward!” the bunny sings,
With tidy options on its wings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description lacks the template's issue reference, structured rationale/key changes, and the required checklist section.	Add a Fixes #XXXX reference if applicable, a structured Description with goal/approach/key changes, and the required test/docs checklist items.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is clear and matches the main change: adding no-store cache-control for S3 uploads.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/storage/metrics_layer.rs (1)

344-383: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise the wrapper upload methods in tests.

These tests call set_cache_control directly, so a regression in put_opts or put_multipart_opts could leave real uploads without the header while all tests still pass. Add a recording-store or request-level test covering both methods for JSON and Parquet paths. object_store exposes attributes through both option types, and S3 maps Attribute::CacheControl to the request header. (docs.rs)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/metrics_layer.rs` around lines 344 - 383, Add request-level tests
that exercise MetricLayer’s put_opts and put_multipart_opts methods for both
JSON and Parquet paths, using a recording or inspecting store to verify
propagated attributes. Confirm JSON uploads include Attribute::CacheControl set
to no-store while Parquet uploads omit it, covering both wrapper methods rather
than calling set_cache_control directly.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/storage/metrics_layer.rs`:
- Around line 344-383: Add request-level tests that exercise MetricLayer’s
put_opts and put_multipart_opts methods for both JSON and Parquet paths, using a
recording or inspecting store to verify propagated attributes. Confirm JSON
uploads include Attribute::CacheControl set to no-store while Parquet uploads
omit it, covering both wrapper methods rather than calling set_cache_control
directly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 25503237-a455-4c7c-a224-eb193b790edc

📥 Commits

Reviewing files that changed from the base of the PR and between dc8ff16 and 592e355.

📒 Files selected for processing (2)

• src/storage/metrics_layer.rs
• src/storage/s3.rs