-
Notifications
You must be signed in to change notification settings - Fork 25.2k
HLRC: Add Get Lifecycle Policy API to HLRC #33323
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
Conversation
Adds Request and Reponse classes for accessing lifecycle policies. Changes existing tests to use these classes where appropriate. Sets up SPI configuration to allow parsing *Actions from XContent.
Pinging @elastic/es-core-infra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but maybe @hub-cap should also have a look before we merge since this adds some SPI stuff I'm not so familiar with
ill have a look today |
+1 to using SPI for the custom XContent. cc @talevy who was also asking me about this the other day. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to create a ILM SPI provider instead of a generic one, or else I might end up splitting it anyway :D
} | ||
|
||
@Override | ||
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this? I dont see it being used anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used to leverage AbstractXContentTestCase
for testing parsing in GetLifecyclePolicyResponseTests
. If there's a better way of doing that, please let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont have a good answer for this yet. While we can totally test this, the value in the server -> client.fromXContent is greater than just testing the client.toXContent -> client.fromXContent. I think these kinds of tests are not really providing much value, and we also test the former in the IT tests.
@elasticmachine retest this please The failure is not related to anything this PR touches and does not reproduce locally. |
Adds Request and Reponse classes for accessing lifecycle policies. Changes existing tests to use these classes where appropriate. Sets up SPI configuration to allow parsing *Actions from XContent.
Adds Request and Reponse classes for accessing lifecycle policies.
Changes existing tests to use these classes where appropriate.
Sets up SPI configuration to allow parsing *Actions from XContent.
Relates to #33100