Add rest and transport plumbing #40997
Conversation
Also wired up a bit more of the plugin and tested against a live cluster.
hub-cap
added
WIP
:Data Management/Indices APIs
|
Pinging @elastic/es-core-features |
|
This is a work in progress. its not close to done, but I wanted to get something that other people can work against. I still need some tests at the very minimum. I also want to make sure the naming of things is good, so thats something I would like to discuss as well. I am also not 100% sure what label to put it under. |
|
|
||
| import java.io.IOException; | ||
|
|
||
| public class RestGenEnrichPolicyAction extends BaseRestHandler { |
There was a problem hiding this comment.
RestGenEnrichPolicyAction -> RestGetEnrichPolicyAction
|
|
||
| public RestGenEnrichPolicyAction(final Settings settings, final RestController controller) { | ||
| super(settings); | ||
| controller.registerHandler(RestRequest.Method.GET, "/{index}/_enrich", this); |
There was a problem hiding this comment.
I think we should have the following path structure: /_enrich/policy/{name}
There was a problem hiding this comment.
ok, so completely remove the {index} and only put it in the payload of the policy itself, right?
There was a problem hiding this comment.
So the payload would contain the: source index, decorate fields etc.
The name (or id) of the policy would exist in the url path.
|
Should we adding this api to the enrich plugin instead? I think the only reason that we would want to add this to xpack-core is for transport client support (at least we did in the past iirc)? I don't think we should be adding transport client support and just hlrc support, so we can keep this inside the enrich plugin instead? |
jasontedor
added
:Data Management/Ingest Node
What would happen if another X-Pack plugin wanted to use this functionality? For example, at the moment ML calls the This is all very theoretical at the moment, but may be something to consider. |
|
@droberts195 Good point. I did not consider this. Maybe we should consider this on an api by api basis. I don't think that ml will need to depend on all of enrich's APIs? So if in the future ml needs to use specific APIs then we can just move those specific api(s) to xpack-core? I do lean towards moving as much as possible code into the specific xpack plugin directories. |
Since transport client, even tho its deprecated, is supported for all of 7, wouldn't that mean that we should support all of the calls that we can do even if the HLRC ones exist? I recently told @dimitris-athanasiou this was the case with some new stuff he was working on, so im CC'ing him on this :) |
I don't think we have to support new APIs in the transport client, now that it has been deprecated and because there is an alternative. |
Yes but the alternative might require a complete app rewrite if you want to use the rest client instead of the transport client for the entire 7.x series, or have 2 different ways to access a cluster depending on the APIs that you care to use. This seems painful for our users. |
The alternative is allow user to add new code against an API that will taken away in 8.0.0. There will be pain either way. a) code against deprecated now and re-write for 8.0.0 or b) start using the HLRC now. I think the former is actually more painful. This argument is only for net-new features added in 7.x (like this one). I do believe that minor changes for features pre-existing in 6.x should continue support for the TransportClient. |
This allows the transport client use this class in enrich APIs. Relates to elastic#40997
This allows the transport client use this class in enrich APIs. Relates to #40997
This allows the transport client use this class in enrich APIs. Relates to #40997
Also wired up a bit more of the plugin and tested against a live
cluster.