Skip to content
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

Move SnapshotsRecoveryPlannerService to its own x-pack plugin #79637

Merged
merged 6 commits into from
Oct 26, 2021

Conversation

fcofdez
Copy link
Contributor

@fcofdez fcofdez commented Oct 21, 2021

No description provided.

@fcofdez
Copy link
Contributor Author

fcofdez commented Oct 22, 2021

@henningandersen I'm leaving this as a draft since I think there's a problem in the task wiring for the qa tests, but it would be nice to get this reviewed. The changes are really similar to #78928 the change-set is large since automatic formatting is applied to new projects and therefore it has changed everything :D. I copied the relevant tests from IndexRecoveryIT to IndexRecoveryWithSnapshotsIT.java since it seems like it's not a good idea to expose the server test artefacts to x-pack plugins.

@fcofdez fcofdez marked this pull request as ready for review October 26, 2021 12:58
@fcofdez
Copy link
Contributor Author

fcofdez commented Oct 26, 2021

@elasticsearchmachine test this please

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM, provided CI is happy, also when running the s3/gc/azure tests.

throw new IllegalStateException("A single RecoveryPlannerPlugin was expected but got: " + recoveryPlannerPlugins);
}

final ShardSnapshotsService shardSnapshotsService = new ShardSnapshotsService(client,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could also move the creation of this service and the service into the x-pack module as a follow-up? No need to do it now.

@fcofdez
Copy link
Contributor Author

fcofdez commented Oct 26, 2021

@fcofdez fcofdez merged commit 8430a58 into elastic:master Oct 26, 2021
@fcofdez fcofdez added the :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. label Oct 26, 2021
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team label Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. >non-issue Team:Distributed Meta label for distributed team v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants