Skip to content

Conversation

DaveCTurner
Copy link
Contributor

Today the ListObjects implementation in S3HttpHandler will put all
the common prefixes in a single <CommonPrefixes> container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.

Today the `ListObjects` implementation in `S3HttpHandler` will put all
the common prefixes in a single `<CommonPrefixes>` container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.19.0 v9.1.0 labels Apr 2, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Apr 2, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@DaveCTurner DaveCTurner requested a review from ywangd April 2, 2025 20:53
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner added the auto-backport Automatically create backport pull requests when merged label Apr 3, 2025
@DaveCTurner DaveCTurner merged commit 9b353f6 into elastic:main Apr 3, 2025
17 checks passed
@DaveCTurner DaveCTurner deleted the 2025/04/02/S3HttpHandler-CommonPrefixes branch April 3, 2025 06:55
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Apr 3, 2025
Today the `ListObjects` implementation in `S3HttpHandler` will put all
the common prefixes in a single `<CommonPrefixes>` container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

elasticsearchmachine pushed a commit that referenced this pull request Apr 3, 2025
Today the `ListObjects` implementation in `S3HttpHandler` will put all
the common prefixes in a single `<CommonPrefixes>` container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.
andreidan pushed a commit to andreidan/elasticsearch that referenced this pull request Apr 9, 2025
Today the `ListObjects` implementation in `S3HttpHandler` will put all
the common prefixes in a single `<CommonPrefixes>` container, but in
fact the real S3 gives each one its own container. The v1 SDK is lenient
and accepts either, but the v2 SDK requires us to do this correctly.
This commit fixes the test fixture to match the behaviour of the real
S3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination Meta label for Distributed Coordination team >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants