HLRC: Adding Update datafeed API#34882
Conversation
|
Pinging @elastic/es-core-infra |
|
Pinging @elastic/ml-core |
dimitris-athanasiou
left a comment
There was a problem hiding this comment.
Looks good. Left a few minor comments.
|
|
||
| static Request updateDatafeed(UpdateDatafeedRequest updateDatafeedRequest) throws IOException { | ||
| String endpoint = new EndpointBuilder() | ||
| .addPathPart("").addPathPartAsIs("_xpack") |
There was a problem hiding this comment.
The first empty part seems redundant
There was a problem hiding this comment.
definitely :D copy paste error
| } | ||
|
|
||
| /** | ||
| * Updated a Machine Learning Datafeed |
There was a problem hiding this comment.
typo: Updated -> Updates
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Updates a {@link org.elasticsearch.client.ml.datafeed.DatafeedConfig} with the passed {@link DatafeedUpdate} |
There was a problem hiding this comment.
This comment implies this class is the action. Reword to suit it is a request?
| machineLearningClient::updateDatafeed, | ||
| machineLearningClient::updateDatafeedAsync); | ||
|
|
||
| DatafeedConfig updatedDatafeed= response.getResponse(); |
There was a problem hiding this comment.
nit: missing space before equals
| <1> The updated configuration of the {ml} datafeed | ||
|
|
||
| [id="{upid}-{api}-config"] | ||
| ==== Updated Datafeed Configuration |
There was a problem hiding this comment.
This title/section might be unnecessary. Check update-job documentation. We could have a similar structure here unless you dislike that one.
dimitris-athanasiou
left a comment
There was a problem hiding this comment.
LGTM apart from a little typo
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Requests an updates to a {@link org.elasticsearch.client.ml.datafeed.DatafeedConfig} with the passed {@link DatafeedUpdate} |
There was a problem hiding this comment.
nit: updates -> update
* HLRC: Adding Update datafeed API * Addressing unused import * Adjusting docs and fixing minor comments * fixing comment
* HLRC: Adding Update datafeed API * Addressing unused import * Adjusting docs and fixing minor comments * fixing comment
HLRC: ML Adds the Update Datafeed API
Relates to #29827