HLRC: Add ML get influencers API #33389
Conversation
|
Pinging @elastic/es-core-infra |
|
Pinging @elastic/ml-core |
| public static final ParseField SORT = new ParseField("sort"); | ||
| public static final ParseField DESCENDING = new ParseField("desc"); | ||
|
|
||
| public static final ObjectParser<GetInfluencersRequest, Void> PARSER = new ObjectParser<>("get_influencers_request", |
There was a problem hiding this comment.
This should be a ConstructingObjectParser so that the private empty ctr can be removed.
| private String sort; | ||
| private Boolean descending; | ||
|
|
||
| private GetInfluencersRequest() {} |
There was a problem hiding this comment.
Not necessary with ConstructingObjectParser
| /** | ||
| * Sets the value of "end" which is a timestamp. | ||
| * Only influencers whose timestamp is before the "end" value will be returned. | ||
| * @param end value of "end" to be set |
There was a problem hiding this comment.
Knowing the supported time formats would be helpful for the user. (this goes for all the time fields in this object)
There was a problem hiding this comment.
I have also taken the opportunity to fix this in other get result request objects.
| protected GetRecordsRequest createTestInstance() { | ||
| GetRecordsRequest request = new GetRecordsRequest(ESTestCase.randomAlphaOfLengthBetween(1, 20)); | ||
|
|
||
| if (ESTestCase.randomBoolean()) { |
There was a problem hiding this comment.
references to ESTestCast can probably be removed as AbstractXContentTestCase extends that class.
| @Override | ||
| protected GetInfluencersResponse createTestInstance() { | ||
| String jobId = ESTestCase.randomAlphaOfLength(20); | ||
| int listSize = ESTestCase.randomInt(10); |
There was a problem hiding this comment.
Again, here references to ESTestCast can probably be removed as AbstractXContentTestCase extends that class.
| @@ -25,11 +25,11 @@ | |||
|
|
|||
| import java.io.IOException; | |||
|
|
|||
| public class GetRecordsRequestTests extends AbstractXContentTestCase<GetRecordsRequest> { | |||
| public class GetRecordsRequestTests extends AbstractXContentTestCase<GetInfluencersRequest> { | |||
There was a problem hiding this comment.
GetRecordsRequestTests should still reference GetRecordsRequest correct?
This smells like a find/replace error.
| [[java-rest-high-x-pack-ml-get-influencers]] | ||
| === Get Influencers API | ||
|
|
||
| The Get Records API retrieves one or more influencer results. |
There was a problem hiding this comment.
Copy paste and find/replace error. Should be Get Influencers API
| protected GetRecordsRequest doParseInstance(XContentParser parser) throws IOException { | ||
| return GetRecordsRequest.PARSER.apply(parser, null); | ||
| protected GetInfluencersRequest doParseInstance(XContentParser parser) throws IOException { | ||
| return GetInfluencersRequest.PARSER.apply(parser, null); |
There was a problem hiding this comment.
Mentioned above, this seems like a find/replace error after a copy paste.
|
@benwtrent All good points! Pushed a commit addressing all of them. |
| PARSER.declareBoolean(GetInfluencersRequest::setDescending, DESCENDING); | ||
| } | ||
|
|
||
| private String jobId; |
* master: Fix deprecated setting specializations (elastic#33412) HLRC: split cluster request converters (elastic#33400) HLRC: Add ML get influencers API (elastic#33389) Add conditional token filter to elasticsearch (elastic#31958) Build: Merge xpack checkstyle config into core (elastic#33399) Disable IndexRecoveryIT.testRerouteRecovery. INGEST: Implement Drop Processor (elastic#32278) [ML] Add field stats to log structure finder (elastic#33351) Add interval response parameter to AutoDateInterval histogram (elastic#33254) MINOR+CORE: Remove Dead Methods ClusterService (elastic#33346)
Relates #29827