Skip to content

Conversation

@crepererum
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

delete is just delete_stream with a single element.

What changes are included in this PR?

  1. move method to extension trait
  2. remove no-longer-required implementations

Are there any user-facing changes?

Breaking: delete moved from ObjectStore to ObjectStoreExt.

@crepererum crepererum force-pushed the crepererum/delete-to-ext branch from f32e43a to cf8f027 Compare November 20, 2025 10:48
@alamb
Copy link
Contributor

alamb commented Nov 25, 2025

@crepererum noted that it turns out that the semantics are subtly different in some error case, though it isn't clear if this difference is purpose.

The normal delete is really just a bulk delete with a single entry.

Part of apache#385.
@crepererum crepererum force-pushed the crepererum/delete-to-ext branch from cf8f027 to 9ff5539 Compare December 12, 2025 10:25
@crepererum
Copy link
Contributor Author

@alamb I've fixed the difference. The AWS tests now pass the same way as before.

@crepererum crepererum marked this pull request as ready for review December 12, 2025 10:34
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Makes sense to me -- thank you @crepererum

FYI @kylebarron

I think once this PR is merged, we can make a release candidate

store: "ext",
source: "`delete_stream` was supposed to yield once but didn't".into(),
})?;
if stream.next().await.is_some() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@crepererum crepererum merged commit 92b1378 into apache:main Dec 12, 2025
8 checks passed
@crepererum crepererum deleted the crepererum/delete-to-ext branch December 12, 2025 14:05
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