Skip to content

Refactor some ILM tests to not use deprecated methods #131417

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nielsbauman
Copy link
Contributor

And updates MetadataMigrateToDataTiersRoutingService to accept a ProjectState instead of a ProjectResolver.

And updates `MetadataMigrateToDataTiersRoutingService` to accept a
`ProjectState` instead of a `ProjectResolver`.
@nielsbauman nielsbauman added >test Issues or PRs that are addressing/adding tests :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Jul 17, 2025
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team v9.2.0 labels Jul 17, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Member

@PeteGillinElastic PeteGillinElastic left a comment

Choose a reason for hiding this comment

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

LGTM as far as the refactoring goes, but I do have a question around what this code is doing.

@@ -245,7 +244,7 @@ public static Tuple<ClusterState, MigratedEntities> migrateToDataTiersRouting(
attribute
);
return Tuple.tuple(
ClusterState.builder(currentState).metadata(mb).putProjectMetadata(newProjectMetadataBuilder).build(),
ClusterState.builder(currentState.cluster()).metadata(mb).putProjectMetadata(newProjectMetadataBuilder).build(),
Copy link
Member

Choose a reason for hiding this comment

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

Your change here seems like a straight refactoring, so that's fine, but I have a wider question. This code is working at project scope, but it's also mutating state at the cluster scope (removing the ENFORCE_DEFAULT_TIER_PREFERENCE setting). That can't be safe, can it? Do we just not care because it's ILM? If so, it feels like we should use your annotation to call that out. If we plan to make it safe, is that tracked somewhere?

@PeteGillinElastic
Copy link
Member

Nit: I just noticed you applied the >test label. That doesn't seem right, since we're touching production code. More of a >non-issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants