migrate allocate action pojo/xcontent to xpack.protocol #32853
Conversation
This splits out the AllocateAction into some parts xpack-protocol so that it can be re-usable in the client.
talevy
added
the
:Data Management/ILM+SLM
|
Pinging @elastic/es-core-infra |
|
@colings86 thanks, don't want to forget about that. Client isn't using it yet, so there are no tests to catch that. I'll add it in |
|
updated with it in. will be used soon! |
| // ILM - Client - Lifecycle Actions | ||
| new NamedXContentRegistry.Entry(org.elasticsearch.protocol.xpack.indexlifecycle.AllocateAction.class, | ||
| new ParseField(org.elasticsearch.protocol.xpack.indexlifecycle.AllocateAction.NAME), | ||
| org.elasticsearch.protocol.xpack.indexlifecycle.AllocateAction::parse) |
There was a problem hiding this comment.
I don't think this is going to work, we need to register the categoryClass as something more generic than the specific action because when we are deserialising the Phase we don't know what specific action we are deserialising (hence why we use the NamedXContentRegistry. I think we need an interface equivalent to LifecycleAction but on the client side.
Also, we will want to move the registering of the server action classes to IndexLifecycle since the client won't need access to the server side objects (this can be done in a clean up PR after the client objects are there if you want but its important we don't forget to do it).
There was a problem hiding this comment.
this is what happens with unused code :)
|
thanks for the review @colings86, I've added a LifecycleAction interface. I preferred to leave it blank, rather than consolidate it with the shared ToXContentObject requirement. I'd rather have the transparency there |
|
nevermind. one second. early morning. this shouldn't be an interface :) |
| import org.elasticsearch.common.xcontent.NamedXContentRegistry; | ||
|
|
||
| /** | ||
| * Parent class for the client-side ilm actions for the {@link NamedXContentRegistry} |
There was a problem hiding this comment.
nit: I think this should be worded differently given its public:
Marker interface for index lifecycle management actions
Reasons:
- This is a marker interface not a parent class
- The fact that its a client side copy is an implementation detail that client users should not need to know about
- The fact that we use it to have a common category class for the NamedXContentRegistry is an implementation detail I don't think clients should need to consider
This splits out the AllocateAction into some parts xpack-protocol
so that it can be re-usable in the client.