Skip to content

Conversation

@prwhelan
Copy link
Member

Overriding version support to include the 8.19 patch version.

Overriding version support to include the 8.19 patch version.
@prwhelan prwhelan added >non-issue :ml Machine learning Team:ML Meta label for the ML team v9.1.0 labels Jun 25, 2025
@prwhelan prwhelan marked this pull request as ready for review June 25, 2025 18:06
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@prwhelan prwhelan changed the title [ML] SageMaker supports 8.19 + 9.1 clusters [ML] Support 8.19 + 9.1 clusters for all new inference providers Jun 25, 2025

@Override
public boolean supportsVersion(TransportVersion version) {
return ServiceSettings.super.supportsVersion(version)
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be more clear to do version.onOrAfter(ML_INFERENCE_HUGGING_FACE_RERANK_ADDED) || version.isPatchFrom(ML_INFERENCE_HUGGING_FACE_RERANK_ADDED_8_19). Something like that.

Copy link
Member

Choose a reason for hiding this comment

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

It's the same thing - but it's easier to scan it and make sure it's the same thing.

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM


@Override
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.AMAZON_BEDROCK_TASK_SETTINGS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional/nitpick: I would change this to

throw Exception("should never be called when supportsVersion is used")

or so (see other classes).

Copy link
Member Author

Choose a reason for hiding this comment

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

spoke offline - i agree with this but i'm scared to change to exceptions in case there is something in prod using this, so to keep 9.1+ clusters alive and fail in non-prod, we added assertions

@prwhelan prwhelan added the auto-backport Automatically create backport pull requests when merged label Jun 26, 2025
@prwhelan prwhelan merged commit 9cb30cc into elastic:main Jun 26, 2025
32 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

The backport operation could not be completed due to the following error:

There are no branches to backport to. Aborting.

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

prwhelan added a commit to prwhelan/elasticsearch that referenced this pull request Jun 26, 2025
…stic#130033)

Overriding version support to include the 8.19 patch version.
elasticsearchmachine pushed a commit that referenced this pull request Jun 27, 2025
…0033) (#130124)

Overriding version support to include the 8.19 patch version.
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
…stic#130033)

Overriding version support to include the 8.19 patch version.
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 backport pending :ml Machine learning >non-issue Team:ML Meta label for the ML team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants