HLRC: Add ML Get Buckets API #33056
HLRC: Add ML Get Buckets API #33056
Conversation
|
Pinging @elastic/es-core-infra |
|
Pinging @elastic/ml-core |
| * For additional info | ||
| * see <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/ml-get-bucket.html">ML GET buckets documentation</a> | ||
| * | ||
| * @param request the request |
There was a problem hiding this comment.
nit: capitalise 'The'. Same in the comment for getBucketsAsync
There was a problem hiding this comment.
Actually reviewing further down the lower case start of the parameter description is more consistent. If that is the preferred style change 'Additional' on the line below and in the async version comments
There was a problem hiding this comment.
it's just vastly inconsistent :-(
There was a problem hiding this comment.
I like capitalized as it separates better from the param name. I'll capitalize and cross my fingers.
| GetBucketsResponse response = execute(request, machineLearningClient::getBuckets, machineLearningClient::getBucketsAsync); | ||
|
|
||
| assertThat(response.count(), equalTo(3L)); | ||
| assertThat(response.buckets().get(0).getTimestamp().getTime(), equalTo(1533081600000L)); |
There was a problem hiding this comment.
Nit: extract 1533081600000L to a constant FIRST_BUCKET_TIME as the literal is used several times
| assertThat(response.buckets().size(), equalTo(3)); | ||
| assertThat(response.buckets().get(0).getTimestamp().getTime(), equalTo(1533081600000L + 3 * 3600000L)); | ||
| assertThat(response.buckets().get(1).getTimestamp().getTime(), equalTo(1533081600000L + 4 * 3600000L)); | ||
| assertThat(response.buckets().get(2).getTimestamp().getTime(), equalTo(1533081600000L + 5 * + 3600000L)); |
There was a problem hiding this comment.
nit: extra plus in + 5 * + 3600000L
| } | ||
|
|
||
| /** | ||
| * Sets the value of "excude_interim". |
There was a problem hiding this comment.
excude_interim - missing 'l' -> exclude_interim
There was a problem hiding this comment.
Misspelled in the @param tag also
|
|
||
| /** | ||
| * Constructs paging parameters | ||
| * @param from skips the specified number of buckets. When {@code null} the default value will be used. |
There was a problem hiding this comment.
from skips the specified number of buckets
This class will be reused won't it so not necessarily buckets
| IndexRequest indexRequest = new IndexRequest(RESULTS_INDEX, DOC); | ||
| double bucketScore = randomDoubleBetween(0.0, 100.0, true); | ||
| bucketStats.report(bucketScore); | ||
| indexRequest.source("{\"job_id\":\"" + JOB_ID + "\", \"result_type\":\"bucket\", \"timestamp\": " + timestamp + "," + |
There was a problem hiding this comment.
Any reason why you prefer to build the string representation rather than constructing a Bucket object converting it to a ByteReference and use the overload of indexRequest.source?
There was a problem hiding this comment.
The constructor and setter methods of Bucket are package protected. I like that as it means the users cannot modify a bucket.
* es/master: (62 commits) [DOCS] Add docs for Application Privileges (#32635) Add versions 5.6.12 and 6.4.1 Do NOT allow termvectors on nested fields (#32728) [Rollup] Return empty response when aggs are missing (#32796) [TEST] Add some ACL yaml tests for Rollup (#33035) Move non duplicated actions back into xpack core (#32952) Test fix - GraphExploreResponseTests should not randomise array elements Closes #33086 Use `addIfAbsent` instead of checking if an element is contained TESTS: Fix Random Fail in MockTcpTransportTests (#33061) HLRC: Fix Compile Error From Missing Throws (#33083) [DOCS] Remove reload password from docs cf. #32889 HLRC: Add ML Get Buckets API (#33056) Watcher: Improve error messages for CronEvalTool (#32800) Search: Support of wildcard on docvalue_fields (#32980) Change query field expansion (#33020) INGEST: Cleanup Redundant Put Method (#33034) SQL: skip uppercasing/lowercasing function tests for AZ locales as well (#32910) Fix the default pom file name (#33063) Switch ml basic tests to new style Requests (#32483) Switch some watcher tests to new style Requests (#33044) ...
* es/6.x: (58 commits) [DOCS] Add docs for Application Privileges (#32635) Add versions 5.6.12 and 6.4.1 [Rollup] Return empty response when aggs are missing (#32796) [TEST] Add some ACL yaml tests for Rollup (#33035) Test fix - GraphExploreResponseTests should not randomise array elements Closes #33086 Use `addIfAbsent` instead of checking if an element is contained HLRC: Fix Compile Error From Missing Throws (#33083) [DOCS] Remove reload password from docs cf. #32889 Use a dedicated ConnectionManger for RemoteClusterConnection (#32988) Watcher: Improve error messages for CronEvalTool (#32800) HLRC: Add ML Get Buckets API (#33056) Change query field expansion (#33020) Search: Support of wildcard on docvalue_fields (#32980) Add beta label to MSI on install Elasticsearch page (#28126) SQL: skip uppercasing/lowercasing function tests for AZ locales as well (#32910) [DOCS] Drafts Elasticsearch 6.4.0 release notes (#33039) Fix the default pom file name (#33063) Fix backport of switch ml basic tests to new style Requests (#32483) Switch ml basic tests to new style Requests (#32483) Switch some watcher tests to new style Requests (#33044) ...
Relates #29827