Skip to content

[ML] Hide internal Job update options from the REST API #30537

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.

Sign up for GitHub

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 2 commits into from
May 14, 2018
Merged

[ML] Hide internal Job update options from the REST API #30537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40be8c3
Add JobVersion to update
davidkyle May 10, 2018
2dc1cda
Change yml tests to not use secret job update settings
davidkyle May 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateJobAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public PutJobAction.Response newResponse() {
public static class Request extends AcknowledgedRequest<UpdateJobAction.Request> implements ToXContentObject {

public static UpdateJobAction.Request parseRequest(String jobId, XContentParser parser) {
JobUpdate update = JobUpdate.PARSER.apply(parser, null).setJobId(jobId).build();
JobUpdate update = JobUpdate.EXTERNAL_PARSER.apply(parser, null).setJobId(jobId).build();
return new UpdateJobAction.Request(jobId, update);
}

Expand Down
52 changes: 30 additions & 22 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,34 @@
public class JobUpdate implements Writeable, ToXContentObject {
public static final ParseField DETECTORS = new ParseField("detectors");

public static final ConstructingObjectParser<Builder, Void> PARSER = new ConstructingObjectParser<>(
// For internal updates
static final ConstructingObjectParser<Builder, Void> INTERNAL_PARSER = new ConstructingObjectParser<>(
"job_update", args -> new Builder((String) args[0]));

// For parsing REST requests
public static final ConstructingObjectParser<Builder, Void> EXTERNAL_PARSER = new ConstructingObjectParser<>(
"job_update", args -> new Builder((String) args[0]));

static {
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), Job.ID);
PARSER.declareStringArray(Builder::setGroups, Job.GROUPS);
PARSER.declareStringOrNull(Builder::setDescription, Job.DESCRIPTION);
PARSER.declareObjectArray(Builder::setDetectorUpdates, DetectorUpdate.PARSER, DETECTORS);
PARSER.declareObject(Builder::setModelPlotConfig, ModelPlotConfig.CONFIG_PARSER, Job.MODEL_PLOT_CONFIG);
PARSER.declareObject(Builder::setAnalysisLimits, AnalysisLimits.CONFIG_PARSER, Job.ANALYSIS_LIMITS);
PARSER.declareString((builder, val) -> builder.setBackgroundPersistInterval(
TimeValue.parseTimeValue(val, Job.BACKGROUND_PERSIST_INTERVAL.getPreferredName())), Job.BACKGROUND_PERSIST_INTERVAL);
PARSER.declareLong(Builder::setRenormalizationWindowDays, Job.RENORMALIZATION_WINDOW_DAYS);
PARSER.declareLong(Builder::setResultsRetentionDays, Job.RESULTS_RETENTION_DAYS);
PARSER.declareLong(Builder::setModelSnapshotRetentionDays, Job.MODEL_SNAPSHOT_RETENTION_DAYS);
PARSER.declareStringArray(Builder::setCategorizationFilters, AnalysisConfig.CATEGORIZATION_FILTERS);
PARSER.declareField(Builder::setCustomSettings, (p, c) -> p.map(), Job.CUSTOM_SETTINGS, ObjectParser.ValueType.OBJECT);
PARSER.declareString(Builder::setModelSnapshotId, Job.MODEL_SNAPSHOT_ID);
PARSER.declareLong(Builder::setEstablishedModelMemory, Job.ESTABLISHED_MODEL_MEMORY);
PARSER.declareString(Builder::setJobVersion, Job.JOB_VERSION);
for (ConstructingObjectParser<Builder, Void> parser : Arrays.asList(INTERNAL_PARSER, EXTERNAL_PARSER)) {
parser.declareString(ConstructingObjectParser.optionalConstructorArg(), Job.ID);
parser.declareStringArray(Builder::setGroups, Job.GROUPS);
parser.declareStringOrNull(Builder::setDescription, Job.DESCRIPTION);
parser.declareObjectArray(Builder::setDetectorUpdates, DetectorUpdate.PARSER, DETECTORS);
parser.declareObject(Builder::setModelPlotConfig, ModelPlotConfig.CONFIG_PARSER, Job.MODEL_PLOT_CONFIG);
parser.declareObject(Builder::setAnalysisLimits, AnalysisLimits.CONFIG_PARSER, Job.ANALYSIS_LIMITS);
parser.declareString((builder, val) -> builder.setBackgroundPersistInterval(
TimeValue.parseTimeValue(val, Job.BACKGROUND_PERSIST_INTERVAL.getPreferredName())), Job.BACKGROUND_PERSIST_INTERVAL);
parser.declareLong(Builder::setRenormalizationWindowDays, Job.RENORMALIZATION_WINDOW_DAYS);
parser.declareLong(Builder::setResultsRetentionDays, Job.RESULTS_RETENTION_DAYS);
parser.declareLong(Builder::setModelSnapshotRetentionDays, Job.MODEL_SNAPSHOT_RETENTION_DAYS);
parser.declareStringArray(Builder::setCategorizationFilters, AnalysisConfig.CATEGORIZATION_FILTERS);
parser.declareField(Builder::setCustomSettings, (p, c) -> p.map(), Job.CUSTOM_SETTINGS, ObjectParser.ValueType.OBJECT);
}
// These fields should not be set by a REST request
INTERNAL_PARSER.declareString(Builder::setModelSnapshotId, Job.MODEL_SNAPSHOT_ID);
INTERNAL_PARSER.declareLong(Builder::setEstablishedModelMemory, Job.ESTABLISHED_MODEL_MEMORY);
INTERNAL_PARSER.declareString(Builder::setJobVersion, Job.JOB_VERSION);
}

private final String jobId;
Expand Down Expand Up @@ -224,14 +232,14 @@ public Long getEstablishedModelMemory() {
return establishedModelMemory;
}

public boolean isAutodetectProcessUpdate() {
return modelPlotConfig != null || detectorUpdates != null;
}

public Version getJobVersion() {
return jobVersion;
}

public boolean isAutodetectProcessUpdate() {
return modelPlotConfig != null || detectorUpdates != null;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
Expand Down Expand Up @@ -332,7 +340,7 @@ public Set<String> getUpdateFields() {
/**
* Updates {@code source} with the new values in this object returning a new {@link Job}.
*
* @param source Source job to be updated
* @param source Source job to be updated
* @param maxModelMemoryLimit The maximum model memory allowed
* @return A new job equivalent to {@code source} updated.
*/
Expand Down
16 changes: 11 additions & 5 deletions .../plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

public class JobUpdateTests extends AbstractSerializingTestCase<JobUpdate> {

private boolean useInternalParser = randomBoolean();

@Override
protected JobUpdate createTestInstance() {
JobUpdate.Builder update = new JobUpdate.Builder(randomAlphaOfLength(4));
Expand Down Expand Up @@ -84,13 +86,13 @@ protected JobUpdate createTestInstance() {
if (randomBoolean()) {
update.setCustomSettings(Collections.singletonMap(randomAlphaOfLength(10), randomAlphaOfLength(10)));
}
if (randomBoolean()) {
if (useInternalParser && randomBoolean()) {
update.setModelSnapshotId(randomAlphaOfLength(10));
}
if (randomBoolean()) {
if (useInternalParser && randomBoolean()) {
update.setEstablishedModelMemory(randomNonNegativeLong());
}
if (randomBoolean()) {
if (useInternalParser && randomBoolean()) {
update.setJobVersion(randomFrom(Version.CURRENT, Version.V_6_2_0, Version.V_6_1_0));
}

Expand All @@ -104,7 +106,11 @@ protected Writeable.Reader<JobUpdate> instanceReader() {

@Override
protected JobUpdate doParseInstance(XContentParser parser) {
return JobUpdate.PARSER.apply(parser, null).build();
if (useInternalParser) {
return JobUpdate.INTERNAL_PARSER.apply(parser, null).build();
} else {
return JobUpdate.EXTERNAL_PARSER.apply(parser, null).build();
}
}

public void testMergeWithJob() {
Expand Down Expand Up @@ -141,7 +147,7 @@ public void testMergeWithJob() {
JobUpdate update = updateBuilder.build();

Job.Builder jobBuilder = new Job.Builder("foo");
jobBuilder.setGroups(Arrays.asList("group-1"));
jobBuilder.setGroups(Collections.singletonList("group-1"));
Detector.Builder d1 = new Detector.Builder("info_content", "domain");
d1.setOverFieldName("mlcategory");
Detector.Builder d2 = new Detector.Builder("min", "field");
Expand Down
27 changes: 21 additions & 6 deletions x-pack/plugin/src/test/resources/rest-api-spec/test/ml/delete_model_snapshot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,24 @@ setup:
"description": "second",
"latest_record_time_stamp": "2016-06-01T00:00:00Z",
"latest_result_time_stamp": "2016-06-01T00:00:00Z",
"snapshot_doc_count": 3
"snapshot_doc_count": 3,
"model_size_stats": {
"job_id" : "delete-model-snapshot",
"result_type" : "model_size_stats",
"model_bytes" : 0,
"total_by_field_count" : 101,
"total_over_field_count" : 0,
"total_partition_field_count" : 0,
"bucket_allocation_failures_count" : 0,
"memory_status" : "ok",
"log_time" : 1495808248662,
"timestamp" : 1495808248662
},
"quantiles": {
"job_id": "delete-model-snapshot",
"timestamp": 1495808248662,
"quantile_state": "quantiles-1"
}
}

- do:
Expand All @@ -106,12 +123,10 @@ setup:
- do:
headers:
Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser
xpack.ml.update_job:
xpack.ml.revert_model_snapshot:
job_id: delete-model-snapshot
body: >
{
"model_snapshot_id": "active-snapshot"
}
snapshot_id: "active-snapshot"


---
"Test delete snapshot missing snapshotId":
Expand Down