瀏覽代碼

[ML] write deprecation warning when include_model_definition parameter is used (#62834)

for get trained models include_model_definition is now deprecated.

This commit writes a deprecation warning if that parameter is used and suggests the caller to utilize the replacement
Benjamin Trent 5 年之前
父節點
當前提交
4819e14b65

+ 5 - 5
x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/TrainedModelIT.java

@@ -88,7 +88,7 @@ public class TrainedModelIT extends ESRestTestCase {
         assertThat(response, containsString("\"count\":2"));
 
         getModel = client().performRequest(new Request("GET",
-            MachineLearning.BASE_PATH + "inference/a_test_regression_model?human=true&include_model_definition=true"));
+            MachineLearning.BASE_PATH + "inference/a_test_regression_model?human=true&include=definition"));
         assertThat(getModel.getStatusLine().getStatusCode(), equalTo(200));
 
         response = EntityUtils.toString(getModel.getEntity());
@@ -100,7 +100,7 @@ public class TrainedModelIT extends ESRestTestCase {
         assertThat(response, containsString("\"count\":1"));
 
         getModel = client().performRequest(new Request("GET",
-            MachineLearning.BASE_PATH + "inference/a_test_regression_model?decompress_definition=false&include_model_definition=true"));
+            MachineLearning.BASE_PATH + "inference/a_test_regression_model?decompress_definition=false&include=definition"));
         assertThat(getModel.getStatusLine().getStatusCode(), equalTo(200));
 
         response = EntityUtils.toString(getModel.getEntity());
@@ -112,7 +112,7 @@ public class TrainedModelIT extends ESRestTestCase {
 
         ResponseException responseException = expectThrows(ResponseException.class, () ->
             client().performRequest(new Request("GET",
-                MachineLearning.BASE_PATH + "inference/a_test_regression*?human=true&include_model_definition=true")));
+                MachineLearning.BASE_PATH + "inference/a_test_regression*?human=true&include=definition")));
         assertThat(EntityUtils.toString(responseException.getResponse().getEntity()),
             containsString(Messages.INFERENCE_TOO_MANY_DEFINITIONS_REQUESTED));
 
@@ -181,7 +181,7 @@ public class TrainedModelIT extends ESRestTestCase {
 
     public void testGetPrePackagedModels() throws IOException {
         Response getModel = client().performRequest(new Request("GET",
-            MachineLearning.BASE_PATH + "inference/lang_ident_model_1?human=true&include_model_definition=true"));
+            MachineLearning.BASE_PATH + "inference/lang_ident_model_1?human=true&include=definition"));
 
         assertThat(getModel.getStatusLine().getStatusCode(), equalTo(200));
         String response = EntityUtils.toString(getModel.getEntity());
@@ -204,7 +204,7 @@ public class TrainedModelIT extends ESRestTestCase {
         getModel = client().performRequest(new Request("GET",
             MachineLearning.BASE_PATH +
                 "inference/" + modelId +
-                "?include_model_definition=true&decompress_definition=false&for_export=true"));
+                "?include=definition&decompress_definition=false&for_export=true"));
         assertThat(getModel.getStatusLine().getStatusCode(), equalTo(200));
 
         Map<String, Object> exportedModel = entityAsMap(getModel);

+ 14 - 4
x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/inference/RestGetTrainedModelsAction.java

@@ -8,6 +8,7 @@ package org.elasticsearch.xpack.ml.rest.inference;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -36,6 +37,8 @@ import static org.elasticsearch.xpack.core.ml.action.GetTrainedModelsAction.Requ
 
 public class RestGetTrainedModelsAction extends BaseRestHandler {
 
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetTrainedModelsAction.class);
+
     @Override
     public List<Route> routes() {
         return List.of(
@@ -62,11 +65,18 @@ public class RestGetTrainedModelsAction extends BaseRestHandler {
                 restRequest.paramAsStringArray(
                     GetTrainedModelsAction.Request.INCLUDE.getPreferredName(),
                     Strings.EMPTY_ARRAY)));
-        final GetTrainedModelsAction.Request request = restRequest.hasParam(GetTrainedModelsAction.Request.INCLUDE_MODEL_DEFINITION) ?
-            new GetTrainedModelsAction.Request(modelId,
+        final GetTrainedModelsAction.Request request;
+        if (restRequest.hasParam(GetTrainedModelsAction.Request.INCLUDE_MODEL_DEFINITION)) {
+            deprecationLogger.deprecate(
+                GetTrainedModelsAction.Request.INCLUDE_MODEL_DEFINITION,
+                "[{}] parameter is deprecated! Use [include=definition] instead.",
+                GetTrainedModelsAction.Request.INCLUDE_MODEL_DEFINITION);
+            request = new GetTrainedModelsAction.Request(modelId,
                 restRequest.paramAsBoolean(GetTrainedModelsAction.Request.INCLUDE_MODEL_DEFINITION, false),
-                tags) :
-            new GetTrainedModelsAction.Request(modelId, tags, includes);
+                tags);
+        } else {
+            request = new GetTrainedModelsAction.Request(modelId, tags, includes);
+        }
         if (restRequest.hasParam(PageParams.FROM.getPreferredName()) || restRequest.hasParam(PageParams.SIZE.getPreferredName())) {
             request.setPageParams(new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM),
                 restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE)));

+ 7 - 0
x-pack/plugin/src/test/resources/rest-api-spec/api/ml.get_trained_models.json

@@ -39,6 +39,13 @@
         "required":false,
         "description":"A comma-separate list of fields to optionally include. Valid options are 'definition' and 'total_feature_importance'. Default is none."
       },
+      "include_model_definition":{
+        "type":"boolean",
+        "required":false,
+        "description":"Should the full model definition be included in the results. These definitions can be large. So be cautious when including them. Defaults to false.",
+        "default":false,
+        "deprecated": true
+      },
       "decompress_definition":{
         "type":"boolean",
         "required":false,

+ 11 - 0
x-pack/plugin/src/test/resources/rest-api-spec/test/ml/inference_crud.yml

@@ -871,3 +871,14 @@ setup:
   - is_false: trained_model_configs.0.estimated_heap_memory_usage
   - is_false: trained_model_configs.0.estimated_operations
   - is_false: trained_model_configs.0.license_level
+---
+"Test deprecation of include model definition param":
+  - skip:
+      features: "warnings"
+  - do:
+      warnings:
+        - "[include_model_definition] parameter is deprecated! Use [include=definition] instead."
+      ml.get_trained_models:
+        model_id: "a-regression-model-1"
+        include_model_definition: true
+        decompress_definition: false