Skip to content

Conversation

@DaveCTurner
Copy link
Contributor

The s3.client.CLIENT_NAME.protocol setting became unused in #126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
s3.client.CLIENT_NAME.endpoint setting to be a URL that includes a
scheme, so in #127489 we prepend a https:// to the endpoint if needed.
This commit generalizes this slightly so that we prepend http:// if
the endpoint has no scheme and the .protocol setting is set to http.

The `s3.client.CLIENT_NAME.protocol` setting became unused in elastic#126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in elastic#127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label May 6, 2025
@DaveCTurner DaveCTurner added the auto-backport Automatically create backport pull requests when merged label May 6, 2025
Copy link
Contributor

@nicktindall nicktindall 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 merged commit d934a0c into elastic:main May 7, 2025
17 checks passed
@DaveCTurner DaveCTurner deleted the 2025/05/06/aws-sdk-protocol branch May 7, 2025 09:02
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 127744

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 7, 2025
The `s3.client.CLIENT_NAME.protocol` setting became unused in elastic#126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in elastic#127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.

Backport of elastic#127744 to `8.19`
@DaveCTurner
Copy link
Contributor Author

Backport is #127809

elasticsearchmachine pushed a commit that referenced this pull request May 7, 2025
* Reinstate use of S3 `protocol` client setting

The `s3.client.CLIENT_NAME.protocol` setting became unused in #126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in #127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.

Backport of #127744 to `8.19`

* Fix up warning assertions
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request May 9, 2025
The `s3.client.CLIENT_NAME.protocol` setting became unused in elastic#126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in elastic#127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request May 9, 2025
The `s3.client.CLIENT_NAME.protocol` setting became unused in elastic#126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in elastic#127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request May 12, 2025
The `s3.client.CLIENT_NAME.protocol` setting became unused in elastic#126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in elastic#127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.
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 >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants