Skip to content

Use valid REST version when determining capabilities #123864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 7, 2025

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Mar 3, 2025

v8 nodes don't know about v9 REST API. But when doing a capabilities check, with implicit rest version, you just want to know what the current cluster can do. So this does that.

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Mar 3, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@alexey-ivanov-es
Copy link
Contributor

There are already PRs for this ticket: #122613 #122609 doing that slightly differently 😕

@thecoop
Copy link
Member Author

thecoop commented Mar 3, 2025

So, this uses a cluster feature to determine when we should use v8 vs v9 api version by default, and doesn't need any changes on 8.x lines.

@thecoop
Copy link
Member Author

thecoop commented Mar 5, 2025

This could be made more generic by using N and N-1 REST versions rather than hardcoded v8 and v9 - that would remove the need for an UpdateForV10

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This could be made more generic

I agree, and I think making it more generic would make this more robust so we don't need to make future changes (the UpdateForV10 annotation would not be needed).

@thecoop thecoop requested a review from rjernst March 6, 2025 08:58
@thecoop thecoop force-pushed the rest-v9-capabilities branch from 3758cbb to 1e9ec42 Compare March 6, 2025 09:29
@thecoop thecoop added the auto-backport Automatically create backport pull requests when merged label Mar 6, 2025
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@thecoop thecoop merged commit 135b165 into elastic:main Mar 7, 2025
17 checks passed
@thecoop thecoop deleted the rest-v9-capabilities branch March 7, 2025 08:18
thecoop added a commit to thecoop/elasticsearch that referenced this pull request Mar 7, 2025
If the rest api version is not specified, infer the correct one to use from the major versions present on the cluster, determined using features
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0

elasticsearchmachine pushed a commit that referenced this pull request Mar 7, 2025
If the rest api version is not specified, infer the correct one to use from the major versions present on the cluster, determined using features
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Mar 11, 2025
If the rest api version is not specified, infer the correct one to use from the major versions present on the cluster, determined using features
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 :Core/Infra/REST API REST infrastructure and utilities >refactoring Team:Core/Infra Meta label for core/infra team v9.0.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants