From 9a40fff6c8ffeda856ff2fd66c8fb5f24f0dd210 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Mon, 22 Jul 2019 11:32:59 +0200 Subject: [PATCH 1/2] Deprecate the ability to update datafeed's job_id. --- .../client/documentation/MlClientDocumentationIT.java | 7 ++++++- .../xpack/core/ml/datafeed/DatafeedUpdate.java | 8 ++++++++ .../ml/action/UpdateDatafeedActionRequestTests.java | 2 +- .../xpack/core/ml/datafeed/DatafeedUpdateTests.java | 10 +++++----- .../resources/rest-api-spec/test/ml/datafeeds_crud.yml | 5 +++++ 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java index 93f196212ca0c..6e9ce76d73638 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java @@ -32,6 +32,7 @@ import org.elasticsearch.client.MlTestStateCleaner; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.RestHighLevelClient; +import org.elasticsearch.client.WarningFailureException; import org.elasticsearch.client.core.PageParams; import org.elasticsearch.client.indices.CreateIndexRequest; import org.elasticsearch.client.ml.CloseJobRequest; @@ -718,7 +719,7 @@ public void testUpdateDatafeed() throws Exception { DatafeedConfig datafeed = DatafeedConfig.builder(datafeedId, job.getId()).setIndices("foo").build(); client.machineLearning().putDatafeed(new PutDatafeedRequest(datafeed), RequestOptions.DEFAULT); - { + try { AggregatorFactories.Builder aggs = AggregatorFactories.builder(); List scriptFields = Collections.emptyList(); // tag::update-datafeed-config @@ -749,6 +750,10 @@ public void testUpdateDatafeed() throws Exception { DatafeedConfig updatedDatafeed = response.getResponse(); // <1> // end::update-datafeed-response assertThat(updatedDatafeed.getId(), equalTo(datafeedId)); + } catch (WarningFailureException e) { + assertThat( + e.getResponse().getWarnings(), + equalTo(Collections.singletonList("The ability to update datafeed's job_id is deprecated."))); } { DatafeedUpdate datafeedUpdate = new DatafeedUpdate.Builder(datafeedId).setIndices("index_1", "index_2").build(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java index 1a848f86138fc..914bad8b5f764 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java @@ -5,12 +5,14 @@ */ package org.elasticsearch.xpack.core.ml.datafeed; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ObjectParser; @@ -44,6 +46,9 @@ */ public class DatafeedUpdate implements Writeable, ToXContentObject { + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(DatafeedUpdate.class)); + private static final String DEPRECATION_MESSAGE_ON_JOB_ID_UPDATE = "The ability to update datafeed's job_id is deprecated."; + public static final ObjectParser PARSER = new ObjectParser<>("datafeed_update", Builder::new); static { @@ -105,6 +110,9 @@ private DatafeedUpdate(String id, String jobId, TimeValue queryDelay, TimeValue this.scrollSize = scrollSize; this.chunkingConfig = chunkingConfig; this.delayedDataCheckConfig = delayedDataCheckConfig; + if (jobId != null) { + deprecationLogger.deprecated(DEPRECATION_MESSAGE_ON_JOB_ID_UPDATE); + } } public DatafeedUpdate(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateDatafeedActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateDatafeedActionRequestTests.java index 97792d1bbad1d..790cef606837b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateDatafeedActionRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateDatafeedActionRequestTests.java @@ -30,7 +30,7 @@ public void setUpDatafeedId() { @Override protected Request createTestInstance() { - return new Request(DatafeedUpdateTests.createRandomized(datafeedId)); + return new Request(DatafeedUpdateTests.createRandomized(datafeedId, null, false)); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java index 35bff34f93803..5fc2d0f13be9d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java @@ -77,12 +77,12 @@ protected DatafeedUpdate createTestInstance() { } public static DatafeedUpdate createRandomized(String datafeedId) { - return createRandomized(datafeedId, null); + return createRandomized(datafeedId, null, true); } - public static DatafeedUpdate createRandomized(String datafeedId, @Nullable DatafeedConfig datafeed) { + public static DatafeedUpdate createRandomized(String datafeedId, @Nullable DatafeedConfig datafeed, boolean canSetJobId) { DatafeedUpdate.Builder builder = new DatafeedUpdate.Builder(datafeedId); - if (randomBoolean() && datafeed == null) { + if (randomBoolean() && datafeed == null && canSetJobId) { builder.setJobId(randomAlphaOfLength(10)); } if (randomBoolean()) { @@ -276,9 +276,9 @@ public void testApply_GivenRandomUpdates_AssertImmutability() { withoutAggs.setAggProvider(null); datafeed = withoutAggs.build(); } - DatafeedUpdate update = createRandomized(datafeed.getId(), datafeed); + DatafeedUpdate update = createRandomized(datafeed.getId(), datafeed, true); while (update.isNoop(datafeed)) { - update = createRandomized(datafeed.getId(), datafeed); + update = createRandomized(datafeed.getId(), datafeed, true); } DatafeedConfig updatedDatafeed = update.apply(datafeed, Collections.emptyMap()); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml index d8ee4926e97d4..4c9fb9a2f1f0c 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml @@ -203,6 +203,9 @@ setup: --- "Test update datafeed to point to different job": + - skip: + features: "warnings" + - do: ml.put_datafeed: datafeed_id: test-datafeed-1 @@ -214,6 +217,8 @@ setup: } - do: + warnings: + - The ability to update datafeed's job_id is deprecated. ml.update_datafeed: datafeed_id: test-datafeed-1 body: > From 66c7af1a40d29ab07096f63737b92f34dedb9133 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Tue, 23 Jul 2019 08:57:33 +0200 Subject: [PATCH 2/2] Apply review comments --- .../client/MachineLearningIT.java | 28 ++++++++++++++++++- .../MlClientDocumentationIT.java | 10 ++----- .../high-level/ml/update-datafeed.asciidoc | 2 -- .../core/ml/datafeed/DatafeedUpdate.java | 2 +- .../rest-api-spec/test/ml/datafeeds_crud.yml | 2 +- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java index 5a92602d004f1..61b414509cf91 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java @@ -436,9 +436,10 @@ public void testPutDatafeed() throws Exception { } public void testUpdateDatafeed() throws Exception { + MachineLearningClient machineLearningClient = highLevelClient().machineLearning(); + String jobId = randomValidJobId(); Job job = buildJob(jobId); - MachineLearningClient machineLearningClient = highLevelClient().machineLearning(); execute(new PutJobRequest(job), machineLearningClient::putJob, machineLearningClient::putJobAsync); String datafeedId = "datafeed-" + jobId; @@ -462,6 +463,31 @@ public void testUpdateDatafeed() throws Exception { assertThat(datafeedUpdate.getScrollSize(), equalTo(updatedDatafeed.getScrollSize())); } + public void testUpdateDatafeed_UpdatingJobIdIsDeprecated() throws Exception { + MachineLearningClient machineLearningClient = highLevelClient().machineLearning(); + + String jobId = randomValidJobId(); + Job job = buildJob(jobId); + execute(new PutJobRequest(job), machineLearningClient::putJob, machineLearningClient::putJobAsync); + + String anotherJobId = randomValidJobId(); + Job anotherJob = buildJob(anotherJobId); + execute(new PutJobRequest(anotherJob), machineLearningClient::putJob, machineLearningClient::putJobAsync); + + String datafeedId = "datafeed-" + jobId; + DatafeedConfig datafeedConfig = DatafeedConfig.builder(datafeedId, jobId).setIndices("some_data_index").build(); + execute(new PutDatafeedRequest(datafeedConfig), machineLearningClient::putDatafeed, machineLearningClient::putDatafeedAsync); + + DatafeedUpdate datafeedUpdateWithChangedJobId = DatafeedUpdate.builder(datafeedId).setJobId(anotherJobId).build(); + WarningFailureException exception = expectThrows( + WarningFailureException.class, + () -> execute( + new UpdateDatafeedRequest(datafeedUpdateWithChangedJobId), + machineLearningClient::updateDatafeed, + machineLearningClient::updateDatafeedAsync)); + assertThat(exception.getResponse().getWarnings(), contains("The ability to update a datafeed's job_id is deprecated.")); + } + public void testGetDatafeed() throws Exception { String jobId1 = "test-get-datafeed-job-1"; String jobId2 = "test-get-datafeed-job-2"; diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java index 6e9ce76d73638..22e2b97ebeea3 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java @@ -32,7 +32,6 @@ import org.elasticsearch.client.MlTestStateCleaner; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.RestHighLevelClient; -import org.elasticsearch.client.WarningFailureException; import org.elasticsearch.client.core.PageParams; import org.elasticsearch.client.indices.CreateIndexRequest; import org.elasticsearch.client.ml.CloseJobRequest; @@ -719,7 +718,7 @@ public void testUpdateDatafeed() throws Exception { DatafeedConfig datafeed = DatafeedConfig.builder(datafeedId, job.getId()).setIndices("foo").build(); client.machineLearning().putDatafeed(new PutDatafeedRequest(datafeed), RequestOptions.DEFAULT); - try { + { AggregatorFactories.Builder aggs = AggregatorFactories.builder(); List scriptFields = Collections.emptyList(); // tag::update-datafeed-config @@ -731,8 +730,7 @@ public void testUpdateDatafeed() throws Exception { .setQuery(QueryBuilders.matchAllQuery()) // <6> .setQueryDelay(TimeValue.timeValueMinutes(1)) // <7> .setScriptFields(scriptFields) // <8> - .setScrollSize(1000) // <9> - .setJobId("update-datafeed-job"); // <10> + .setScrollSize(1000); // <9> // end::update-datafeed-config // Clearing aggregation to avoid complex validation rules @@ -750,10 +748,6 @@ public void testUpdateDatafeed() throws Exception { DatafeedConfig updatedDatafeed = response.getResponse(); // <1> // end::update-datafeed-response assertThat(updatedDatafeed.getId(), equalTo(datafeedId)); - } catch (WarningFailureException e) { - assertThat( - e.getResponse().getWarnings(), - equalTo(Collections.singletonList("The ability to update datafeed's job_id is deprecated."))); } { DatafeedUpdate datafeedUpdate = new DatafeedUpdate.Builder(datafeedId).setIndices("index_1", "index_2").build(); diff --git a/docs/java-rest/high-level/ml/update-datafeed.asciidoc b/docs/java-rest/high-level/ml/update-datafeed.asciidoc index 86e3a4de336ec..0073bfb7bce6e 100644 --- a/docs/java-rest/high-level/ml/update-datafeed.asciidoc +++ b/docs/java-rest/high-level/ml/update-datafeed.asciidoc @@ -40,8 +40,6 @@ include-tagged::{doc-tests-file}[{api}-config] <7> Optional, the time interval behind real time that data is queried. <8> Optional, allows the use of script fields. <9> Optional, the `size` parameter used in the searches. -<10> Optional, the `jobId` that references the job that the datafeed should be associated with -after the update. include::../execution.asciidoc[] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java index 914bad8b5f764..da9423f6ec7aa 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java @@ -47,7 +47,7 @@ public class DatafeedUpdate implements Writeable, ToXContentObject { private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(DatafeedUpdate.class)); - private static final String DEPRECATION_MESSAGE_ON_JOB_ID_UPDATE = "The ability to update datafeed's job_id is deprecated."; + private static final String DEPRECATION_MESSAGE_ON_JOB_ID_UPDATE = "The ability to update a datafeed's job_id is deprecated."; public static final ObjectParser PARSER = new ObjectParser<>("datafeed_update", Builder::new); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml index 4c9fb9a2f1f0c..91722dded5aa3 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml @@ -218,7 +218,7 @@ setup: - do: warnings: - - The ability to update datafeed's job_id is deprecated. + - The ability to update a datafeed's job_id is deprecated. ml.update_datafeed: datafeed_id: test-datafeed-1 body: >