Skip to content

HLRC: ML Update Job #33392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 6, 2018
Merged

Conversation

benwtrent
Copy link
Member

Adds the ability to update a Machine Learning job through the HLRC.

This relates to (#29827)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Left a few minor comments.

@@ -356,6 +358,38 @@ public void flushJobAsync(FlushJobRequest request, RequestOptions options, Actio
Collections.emptySet());
}

/**
* Updates a Machine Learning {@link org.elasticsearch.client.ml.job.config.Job} properties
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we delete properties?

*/
public class UpdateJobRequest extends ActionRequest implements ToXContentObject {

private JobUpdate update;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make final

[[java-rest-high-x-pack-ml-update-job-request]]
==== Update Job Request

A `UpdateJobRequest` object gets created with an `JobUpdate` object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an -> a

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should just always make The so there is a certain authority and exactness to it :)

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benwtrent benwtrent merged commit 9b6bbc0 into elastic:master Sep 6, 2018
@benwtrent benwtrent deleted the feature/hlrc-ml-update-job branch September 6, 2018 12:18
benwtrent added a commit that referenced this pull request Sep 6, 2018
* HLRC: ML Update Job
@benwtrent benwtrent removed the :ml Machine learning label Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants