Add `Extensions` in `*Result` objects by criccomini · Pull Request #743 · apache/arrow-rs-object-store

criccomini · 2026-06-09T23:26:10Z

Which issue does this PR close?

Closes Support Extensions in *Result objects #740

Rationale for this change

We make extensive use of ObjectStore wrappers that do things like caching, metrics tracking, handle retries, and so on. We've found it would be helpful to use Extensions not just for requests (which are already supported today), but also responses.

One example: we have a CachedObjectStore in SlateDB that we are going to make pluggable. We have an internal metrics wrapper that (among other things) tracks cache hits/misses. Now that we're exposing the cache publicly, we need a way for our wrapper to track whether the reads were a hit or miss. An Extensions in the Result seems appropriate here. Then users that implement a cache can use the extension to signal such information.

This is just one example.

What changes are included in this PR?

Adds a public extensions: http::Extensions field to the existing result types (GetResult, PutResult, ListResult, and PaginatedListResult) mirroring the extensions field already present on the options structs (GetOptions, PutOptions, etc.). This gives ObjectStore implementations (e.g. caching wrappers) a way to return context-specific information, such as cache hit/miss status, to callers.

I did not add Extensions support for Delete/Head as there aren't Result types for these right now. That seemed like a heavier change.

Are there any user-facing changes?

GetResult, PutResult, ListResult, and PaginatedListResult gain a new public field extensions. Code constructing these types with struct literals must add the field (e.g. extensions: Default::default()).
PutResult no longer derives PartialEq/Eq; the manual impls compare e_tag and version only, ignoring extensions.

WARN: This was coded with Claude Fable with some steering from me. I (a human) wrote this PR summary.

criccomini · 2026-06-09T23:27:11Z

+    }
+
+    #[tokio::test]
+    async fn test_response_extensions_propagated() {


This is the only test. Not sure if you want to clutter up the other ones with tests as well. Can do so if desired.

I think we should add coverage of all new code, otherwise we coudl potentially break this feature in some future refactoring but not know

Not sure if you want to clutter up the other ones

I agree this test seems like a lot of boilerplate setup. It would be nice to avoid that

How about writing something similar to

arrow-rs-object-store/src/integration.rs

Lines 51 to 53 in e89f62b

/// Tests basic read/write and listing operations

pub async fn put_get_delete_list(storage: &DynObjectStore) {

And then calling it with an existing fixture, like

arrow-rs-object-store/src/aws/mod.rs

Lines 688 to 689 in a9a83f1

put_get_delete_list(&integration).await;

criccomini · 2026-06-09T23:27:44Z

 #[cfg(any(feature = "aws", feature = "gcp", feature = "azure"))]
 pub(crate) fn get_put_result(
-    headers: &HeaderMap,
+    response: crate::client::HttpResponse,


Different ways to tackle this. LMK if you prefer another approach. Fable initially did a mut here which was kinda gross.

alamb

Thanks @criccomini -- this looks good to me, other than I think we need test coverage for each object store. I left a suggestion, Let me know what you think

alamb · 2026-06-11T20:31:41Z

+    /// response, allowing custom HTTP middleware to propagate information to callers.
+    ///
+    /// These extensions are excluded from [`PartialEq`] and [`Eq`].
+    pub extensions: Extensions,


this is technically a breaking API change (but that is ok as we are about to release a breaking release)

alamb · 2026-06-11T20:38:10Z

+    }
+
+    #[tokio::test]
+    async fn test_response_extensions_propagated() {


I think we should add coverage of all new code, otherwise we coudl potentially break this feature in some future refactoring but not know

Not sure if you want to clutter up the other ones

I agree this test seems like a lot of boilerplate setup. It would be nice to avoid that

How about writing something similar to

arrow-rs-object-store/src/integration.rs

Lines 51 to 53 in e89f62b

/// Tests basic read/write and listing operations

pub async fn put_get_delete_list(storage: &DynObjectStore) {

And then calling it with an existing fixture, like

arrow-rs-object-store/src/aws/mod.rs

Lines 688 to 689 in a9a83f1

put_get_delete_list(&integration).await;

alamb · 2026-06-11T20:45:01Z

+    ///
+    /// HTTP-backed stores in this crate populate this with the extensions of the HTTP
+    /// response, allowing custom HTTP middleware to propagate information to callers.
+    pub extensions: http::Extensions,


Given that ListResult already has Extensions -- does it add additional value to add extensions to the PaginatedListResult as well?

criccomini · 2026-06-11T23:26:22Z

Great! I'll fix things up. 😀

criccomini · 2026-06-12T06:19:34Z

Seems like the failed tests are flakes/unrelated to this PR?

alamb · 2026-06-12T10:58:21Z

Seems like the failed tests are flakes/unrelated to this PR?

Unrelated to the PR for sure. I'll check them out

alamb

Looks great to me -- thank you @criccomini

The MSRV check seems to be failing on installing cargo-msrv so that is unrelated to this PR

alamb · 2026-06-12T11:00:47Z

        s3_encryption(&integration).await;
        put_get_attributes(&integration).await;
        list_paginated(&integration, &integration).await;
+        response_extensions(&integration, true).await;


criccomini added 2 commits June 9, 2026 16:13

add extensions support to results

18d5f25

add extensions to MultiStatus

8467a81

criccomini mentioned this pull request Jun 9, 2026

Support Extensions in *Result objects #740

Closed

fmt

cffa7df

criccomini commented Jun 9, 2026

View reviewed changes

rustdocs

f222443

alamb added the api-change label Jun 11, 2026

alamb reviewed Jun 11, 2026

View reviewed changes

alamb mentioned this pull request Jun 11, 2026

Release object store 0.14.0 (breaking) - Target April 2026 #657

Closed

18 tasks

criccomini added 2 commits June 11, 2026 21:00

get rid of extensions in paginated list result

4f5aa13

add extension result integration tests

804ecf9

alamb approved these changes Jun 12, 2026

View reviewed changes

alamb merged commit 52f5119 into apache:main Jun 12, 2026
13 of 16 checks passed


	/// Tests basic read/write and listing operations
	pub async fn put_get_delete_list(storage: &DynObjectStore) {

Uh oh!

Conversation

criccomini commented Jun 9, 2026 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criccomini commented Jun 11, 2026

Uh oh!

criccomini commented Jun 12, 2026

Uh oh!

alamb commented Jun 12, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

criccomini commented Jun 9, 2026 •

edited by alamb

Loading