Skip to content

Commit 08e4a35

Browse files
author
Tianli Feng
authored
Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs (opensearch-project#2682)
- Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script APIs which have got the parameter. - Add alternative new request parameter `cluster_manager_timeout`. - Add unit tests. Signed-off-by: Tianli Feng <ftianli@amazon.com>
1 parent 3af4300 commit 08e4a35

13 files changed

+145
-12
lines changed

‎rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,15 @@
2828
},
2929
"master_timeout":{
3030
"type":"time",
31-
"description":"Specify timeout for connection to master"
31+
"description":"Specify timeout for connection to master",
32+
"deprecated":{
33+
"version":"2.0.0",
34+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
35+
}
36+
},
37+
"cluster_manager_timeout":{
38+
"type":"time",
39+
"description":"Specify timeout for connection to cluster-manager node"
3240
}
3341
}
3442
}

‎rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,15 @@
2424
"params":{
2525
"master_timeout":{
2626
"type":"time",
27-
"description":"Specify timeout for connection to master"
27+
"description":"Specify timeout for connection to master",
28+
"deprecated":{
29+
"version":"2.0.0",
30+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
31+
}
32+
},
33+
"cluster_manager_timeout":{
34+
"type":"time",
35+
"description":"Specify timeout for connection to cluster-manager node"
2836
}
2937
}
3038
}

‎rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,15 @@
2424
"params":{
2525
"master_timeout":{
2626
"type":"time",
27-
"description":"Explicit operation timeout for connection to master node"
27+
"description":"Explicit operation timeout for connection to master node",
28+
"deprecated":{
29+
"version":"2.0.0",
30+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
31+
}
32+
},
33+
"cluster_manager_timeout":{
34+
"type":"time",
35+
"description":"Explicit operation timeout for connection to cluster-manager node"
2836
},
2937
"timeout":{
3038
"type":"time",

‎rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,15 @@
3030
"params":{
3131
"master_timeout":{
3232
"type":"time",
33-
"description":"Explicit operation timeout for connection to master node"
33+
"description":"Explicit operation timeout for connection to master node",
34+
"deprecated":{
35+
"version":"2.0.0",
36+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
37+
}
38+
},
39+
"cluster_manager_timeout":{
40+
"type":"time",
41+
"description":"Explicit operation timeout for connection to cluster-manager node"
3442
}
3543
}
3644
}

‎rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,15 @@
2424
"params":{
2525
"master_timeout":{
2626
"type":"time",
27-
"description":"Explicit operation timeout for connection to master node"
27+
"description":"Explicit operation timeout for connection to master node",
28+
"deprecated":{
29+
"version":"2.0.0",
30+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
31+
}
32+
},
33+
"cluster_manager_timeout":{
34+
"type":"time",
35+
"description":"Explicit operation timeout for connection to cluster-manager node"
2836
},
2937
"timeout":{
3038
"type":"time",

‎rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json

+9-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@
4646
},
4747
"master_timeout":{
4848
"type":"time",
49-
"description":"Specify timeout for connection to master"
49+
"description":"Specify timeout for connection to master",
50+
"deprecated":{
51+
"version":"2.0.0",
52+
"description":"To support inclusive language, use 'cluster_manager_timeout' instead."
53+
}
54+
},
55+
"cluster_manager_timeout":{
56+
"type":"time",
57+
"description":"Specify timeout for connection to cluster-manager node"
5058
},
5159
"context":{
5260
"type":"string",

‎server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import org.opensearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest;
3535
import org.opensearch.client.node.NodeClient;
36+
import org.opensearch.common.logging.DeprecationLogger;
3637
import org.opensearch.rest.BaseRestHandler;
3738
import org.opensearch.rest.RestRequest;
3839
import org.opensearch.rest.action.RestToXContentListener;
@@ -45,6 +46,8 @@
4546

4647
public class RestDeleteStoredScriptAction extends BaseRestHandler {
4748

49+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeleteStoredScriptAction.class);
50+
4851
@Override
4952
public List<Route> routes() {
5053
return singletonList(new Route(DELETE, "/_scripts/{id}"));
@@ -60,7 +63,10 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
6063
String id = request.param("id");
6164
DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(id);
6265
deleteStoredScriptRequest.timeout(request.paramAsTime("timeout", deleteStoredScriptRequest.timeout()));
63-
deleteStoredScriptRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteStoredScriptRequest.masterNodeTimeout()));
66+
deleteStoredScriptRequest.masterNodeTimeout(
67+
request.paramAsTime("cluster_manager_timeout", deleteStoredScriptRequest.masterNodeTimeout())
68+
);
69+
parseDeprecatedMasterTimeoutParameter(deleteStoredScriptRequest, request, deprecationLogger, getName());
6470

6571
return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new RestToXContentListener<>(channel));
6672
}

‎server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import org.opensearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
3535
import org.opensearch.client.node.NodeClient;
36+
import org.opensearch.common.logging.DeprecationLogger;
3637
import org.opensearch.rest.BaseRestHandler;
3738
import org.opensearch.rest.RestRequest;
3839
import org.opensearch.rest.action.RestStatusToXContentListener;
@@ -45,6 +46,8 @@
4546

4647
public class RestGetStoredScriptAction extends BaseRestHandler {
4748

49+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetStoredScriptAction.class);
50+
4851
@Override
4952
public List<Route> routes() {
5053
return singletonList(new Route(GET, "/_scripts/{id}"));
@@ -59,7 +62,8 @@ public String getName() {
5962
public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException {
6063
String id = request.param("id");
6164
GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id);
62-
getRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getRequest.masterNodeTimeout()));
65+
getRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getRequest.masterNodeTimeout()));
66+
parseDeprecatedMasterTimeoutParameter(getRequest, request, deprecationLogger, getName());
6367
return channel -> client.admin().cluster().getStoredScript(getRequest, new RestStatusToXContentListener<>(channel));
6468
}
6569
}

‎server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.opensearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
3535
import org.opensearch.client.node.NodeClient;
3636
import org.opensearch.common.bytes.BytesReference;
37+
import org.opensearch.common.logging.DeprecationLogger;
3738
import org.opensearch.common.xcontent.XContentType;
3839
import org.opensearch.rest.BaseRestHandler;
3940
import org.opensearch.rest.RestRequest;
@@ -50,6 +51,8 @@
5051

5152
public class RestPutStoredScriptAction extends BaseRestHandler {
5253

54+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutStoredScriptAction.class);
55+
5356
@Override
5457
public List<Route> routes() {
5558
return unmodifiableList(
@@ -76,7 +79,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
7679
StoredScriptSource source = StoredScriptSource.parse(content, xContentType);
7780

7881
PutStoredScriptRequest putRequest = new PutStoredScriptRequest(id, context, content, request.getXContentType(), source);
79-
putRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putRequest.masterNodeTimeout()));
82+
putRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", putRequest.masterNodeTimeout()));
83+
parseDeprecatedMasterTimeoutParameter(putRequest, request, deprecationLogger, getName());
8084
putRequest.timeout(request.paramAsTime("timeout", putRequest.timeout()));
8185
return channel -> client.admin().cluster().putStoredScript(putRequest, new RestToXContentListener<>(channel));
8286
}

‎server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.opensearch.action.ingest.DeletePipelineRequest;
3636
import org.opensearch.client.node.NodeClient;
37+
import org.opensearch.common.logging.DeprecationLogger;
3738
import org.opensearch.rest.BaseRestHandler;
3839
import org.opensearch.rest.RestRequest;
3940
import org.opensearch.rest.action.RestToXContentListener;
@@ -45,6 +46,8 @@
4546
import static org.opensearch.rest.RestRequest.Method.DELETE;
4647

4748
public class RestDeletePipelineAction extends BaseRestHandler {
49+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeletePipelineAction.class);
50+
4851
@Override
4952
public List<Route> routes() {
5053
return singletonList(new Route(DELETE, "/_ingest/pipeline/{id}"));
@@ -58,7 +61,8 @@ public String getName() {
5861
@Override
5962
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
6063
DeletePipelineRequest request = new DeletePipelineRequest(restRequest.param("id"));
61-
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
64+
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
65+
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
6266
request.timeout(restRequest.paramAsTime("timeout", request.timeout()));
6367
return channel -> client.admin().cluster().deletePipeline(request, new RestToXContentListener<>(channel));
6468
}

‎server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.opensearch.action.ingest.GetPipelineRequest;
3636
import org.opensearch.client.node.NodeClient;
3737
import org.opensearch.common.Strings;
38+
import org.opensearch.common.logging.DeprecationLogger;
3839
import org.opensearch.rest.BaseRestHandler;
3940
import org.opensearch.rest.RestRequest;
4041
import org.opensearch.rest.action.RestStatusToXContentListener;
@@ -48,6 +49,8 @@
4849

4950
public class RestGetPipelineAction extends BaseRestHandler {
5051

52+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetPipelineAction.class);
53+
5154
@Override
5255
public List<Route> routes() {
5356
return unmodifiableList(asList(new Route(GET, "/_ingest/pipeline"), new Route(GET, "/_ingest/pipeline/{id}")));
@@ -61,7 +64,8 @@ public String getName() {
6164
@Override
6265
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
6366
GetPipelineRequest request = new GetPipelineRequest(Strings.splitStringByCommaToArray(restRequest.param("id")));
64-
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
67+
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
68+
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
6569
return channel -> client.admin().cluster().getPipeline(request, new RestStatusToXContentListener<>(channel));
6670
}
6771
}

‎server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.opensearch.client.node.NodeClient;
3737
import org.opensearch.common.bytes.BytesReference;
3838
import org.opensearch.common.collect.Tuple;
39+
import org.opensearch.common.logging.DeprecationLogger;
3940
import org.opensearch.common.xcontent.XContentType;
4041
import org.opensearch.rest.BaseRestHandler;
4142
import org.opensearch.rest.RestRequest;
@@ -49,6 +50,8 @@
4950

5051
public class RestPutPipelineAction extends BaseRestHandler {
5152

53+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutPipelineAction.class);
54+
5255
@Override
5356
public List<Route> routes() {
5457
return singletonList(new Route(PUT, "/_ingest/pipeline/{id}"));
@@ -63,7 +66,8 @@ public String getName() {
6366
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
6467
Tuple<XContentType, BytesReference> sourceTuple = restRequest.contentOrSourceParam();
6568
PutPipelineRequest request = new PutPipelineRequest(restRequest.param("id"), sourceTuple.v2(), sourceTuple.v1());
66-
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
69+
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
70+
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
6771
request.timeout(restRequest.paramAsTime("timeout", request.timeout()));
6872
return channel -> client.admin().cluster().putPipeline(request, new RestToXContentListener<>(channel));
6973
}

‎server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java

+59
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@
6363
import org.opensearch.rest.action.admin.cluster.RestRestoreSnapshotAction;
6464
import org.opensearch.rest.action.admin.cluster.RestSnapshotsStatusAction;
6565
import org.opensearch.rest.action.admin.cluster.RestVerifyRepositoryAction;
66+
import org.opensearch.rest.action.admin.cluster.RestDeleteStoredScriptAction;
67+
import org.opensearch.rest.action.admin.cluster.RestGetStoredScriptAction;
68+
import org.opensearch.rest.action.admin.cluster.RestPutStoredScriptAction;
6669
import org.opensearch.rest.action.cat.RestAllocationAction;
6770
import org.opensearch.rest.action.cat.RestRepositoriesAction;
6871
import org.opensearch.rest.action.cat.RestThreadPoolAction;
@@ -76,6 +79,9 @@
7679
import org.opensearch.rest.action.cat.RestPendingClusterTasksAction;
7780
import org.opensearch.rest.action.cat.RestSegmentsAction;
7881
import org.opensearch.rest.action.cat.RestSnapshotAction;
82+
import org.opensearch.rest.action.ingest.RestDeletePipelineAction;
83+
import org.opensearch.rest.action.ingest.RestGetPipelineAction;
84+
import org.opensearch.rest.action.ingest.RestPutPipelineAction;
7985
import org.opensearch.test.OpenSearchTestCase;
8086
import org.opensearch.test.rest.FakeRestRequest;
8187
import org.opensearch.threadpool.TestThreadPool;
@@ -612,6 +618,59 @@ public void testVerifyRepository() {
612618
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
613619
}
614620

621+
public void testDeletePipeline() {
622+
FakeRestRequest request = new FakeRestRequest();
623+
request.params().put("cluster_manager_timeout", "1h");
624+
request.params().put("master_timeout", "3s");
625+
request.params().put("id", "test");
626+
RestDeletePipelineAction action = new RestDeletePipelineAction();
627+
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client));
628+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
629+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
630+
}
631+
632+
public void testGetPipeline() {
633+
RestGetPipelineAction action = new RestGetPipelineAction();
634+
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
635+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
636+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
637+
}
638+
639+
public void testPutPipeline() {
640+
FakeRestRequest request = getFakeRestRequestWithBody();
641+
request.params().put("cluster_manager_timeout", "2m");
642+
request.params().put("master_timeout", "3s");
643+
request.params().put("id", "test");
644+
RestPutPipelineAction action = new RestPutPipelineAction();
645+
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client));
646+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
647+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
648+
}
649+
650+
public void testDeleteStoredScript() {
651+
RestDeleteStoredScriptAction action = new RestDeleteStoredScriptAction();
652+
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
653+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
654+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
655+
}
656+
657+
public void testGetStoredScript() {
658+
RestGetStoredScriptAction action = new RestGetStoredScriptAction();
659+
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
660+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
661+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
662+
}
663+
664+
public void testPutStoredScript() {
665+
RestPutStoredScriptAction action = new RestPutStoredScriptAction();
666+
Exception e = assertThrows(
667+
OpenSearchParseException.class,
668+
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
669+
);
670+
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
671+
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE, "empty templates should no longer be used");
672+
}
673+
615674
private MasterNodeRequest getMasterNodeRequest() {
616675
return new MasterNodeRequest() {
617676
@Override

0 commit comments

Comments
 (0)