Adding ML HLRC wrapper and put_job API call#32726
Conversation
|
Pinging @elastic/ml-core |
| return generator.ofCodePointsLength(random(), 10, 10); | ||
| } | ||
|
|
||
| private static Job buildRandomJob(String jobId) { |
There was a problem hiding this comment.
Most of this PR looks really good, but I wonder if in the future we'd regret creating a totally random job in the integration test (which runs against a real cluster, so no mocking). As more endpoints need to be tested the tests will have to build on each other. For example, you can't test opening a job until you've created one, and you can't test closing a job until you've created one and opened it. So maybe replace this with a buildTestJob(String jobId) method that always builds the same structure of job. This one would be a reasonable choice: https://www.elastic.co/guide/en/elasticsearch/reference/current/ml-put-job.html
There was a problem hiding this comment.
@droberts195 yeah, I had this same debate with myself. I initially had just a consistent job being created. However to better test locally, I ran numerous tests with this random creation and just never reverted back to the consistent job.
I have no qualms making it a semi-statically created job. Will update shortly.
|
Jenkins retest this please |
|
|
||
| DataDescription.Builder dataDescription = new DataDescription.Builder(); | ||
| dataDescription.setTimeFormat(randomFrom(DataDescription.EPOCH_MS, DataDescription.EPOCH)); | ||
| dataDescription.setTimeField(randomAlphaOfLength(10)); |
There was a problem hiding this comment.
To make jobs built with this method reusable when we come to send data to them in other tests, I think it would be better to fix the time format to EPOCH_MS and time field name to a specific value, e.g. "time".
However, since it takes hours to get the PR CI build to complete and we'll be changing this same file in future PRs that implement the other endpoints I'm happy to leave this as-is for now.
* Adding ML HLRC wrapper and put_job API call * Changing integration test job to have consistent stucture
|
heya stumbled upon this merging conflicts in another PR, I noticed that this new API is missing in the docs. Not sure whether this was done on purpose or not but I thought I would raise it. |
|
@javanna yes, I'm going to do the docs for this endpoint in a separate PR because they'll be large and complicated and @benwtrent has only recently joined us. |
colings86
added
>enhancement
:Core/Java High Level REST Client
and removed
:ml
|
Pinging @elastic/es-core-infra |
6 participants
This adds the Machine Learning HLRC wrapper class and creates the first request/response objects necessary for the PUT /job/<job_id> request.