Skip to content
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

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs #2682

Merged
merged 12 commits into from
Apr 13, 2022
Merged

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs #2682

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b5563d3
Deprecate master_timeout parameter in ingest and script APIs
Mar 31, 2022
545db0d
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 2, 2022
70d613a
Change order of an import
Apr 2, 2022
59260e9
Add unit test for ingest and script APIs of parsing cluster_manager_t…
Apr 2, 2022
a606dd1
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 4, 2022
64dc9f3
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 4, 2022
1699174
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 4, 2022
cafd2cc
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 5, 2022
7cc39cc
Merge remote-tracking branch 'upstream/main' into manager-timeout-ing…
Apr 11, 2022
7d0de09
Change promote to support in rest-api-spec 'To support inclusive lang…
Apr 11, 2022
3214cc1
Merge branch 'main' into manager-timeout-ingest-script-api
Apr 12, 2022
a679634
Merge remote-tracking branch 'upstream/main' into manager-timeout-ing…
Apr 12, 2022
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
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@
},
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
Copy link
Member

Choose a reason for hiding this comment

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

It feels to me promote is the not the right work. may be to support inclusive language. or to be more inclusive, use ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your opinion! I will change the word promote to support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in commit 7d0de09

}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Specify timeout for connection to cluster-manager node"
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Specify timeout for connection to cluster-manager node"
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,15 @@
},
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Specify timeout for connection to cluster-manager node"
},
"context":{
"type":"string",
Expand Down
8 changes: 7 additions & 1 deletion .../src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import org.opensearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;
Expand All @@ -45,6 +46,8 @@

public class RestDeleteStoredScriptAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeleteStoredScriptAction.class);

@Override
public List<Route> routes() {
return singletonList(new Route(DELETE, "/_scripts/{id}"));
Expand All @@ -60,7 +63,10 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
String id = request.param("id");
DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(id);
deleteStoredScriptRequest.timeout(request.paramAsTime("timeout", deleteStoredScriptRequest.timeout()));
deleteStoredScriptRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteStoredScriptRequest.masterNodeTimeout()));
deleteStoredScriptRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", deleteStoredScriptRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(deleteStoredScriptRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new RestToXContentListener<>(channel));
}
Expand Down
6 changes: 5 additions & 1 deletion server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import org.opensearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestStatusToXContentListener;
Expand All @@ -45,6 +46,8 @@

public class RestGetStoredScriptAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetStoredScriptAction.class);

@Override
public List<Route> routes() {
return singletonList(new Route(GET, "/_scripts/{id}"));
Expand All @@ -59,7 +62,8 @@ public String getName() {
public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException {
String id = request.param("id");
GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id);
getRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getRequest.masterNodeTimeout()));
getRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(getRequest, request, deprecationLogger, getName());
return channel -> client.admin().cluster().getStoredScript(getRequest, new RestStatusToXContentListener<>(channel));
}
}
6 changes: 5 additions & 1 deletion server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.opensearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.bytes.BytesReference;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
Expand All @@ -50,6 +51,8 @@

public class RestPutStoredScriptAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutStoredScriptAction.class);

@Override
public List<Route> routes() {
return unmodifiableList(
Expand All @@ -76,7 +79,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
StoredScriptSource source = StoredScriptSource.parse(content, xContentType);

PutStoredScriptRequest putRequest = new PutStoredScriptRequest(id, context, content, request.getXContentType(), source);
putRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putRequest.masterNodeTimeout()));
putRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", putRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(putRequest, request, deprecationLogger, getName());
putRequest.timeout(request.paramAsTime("timeout", putRequest.timeout()));
return channel -> client.admin().cluster().putStoredScript(putRequest, new RestToXContentListener<>(channel));
}
Expand Down
6 changes: 5 additions & 1 deletion server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.opensearch.action.ingest.DeletePipelineRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;
Expand All @@ -45,6 +46,8 @@
import static org.opensearch.rest.RestRequest.Method.DELETE;

public class RestDeletePipelineAction extends BaseRestHandler {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeletePipelineAction.class);

@Override
public List<Route> routes() {
return singletonList(new Route(DELETE, "/_ingest/pipeline/{id}"));
Expand All @@ -58,7 +61,8 @@ public String getName() {
@Override
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
DeletePipelineRequest request = new DeletePipelineRequest(restRequest.param("id"));
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
request.timeout(restRequest.paramAsTime("timeout", request.timeout()));
return channel -> client.admin().cluster().deletePipeline(request, new RestToXContentListener<>(channel));
}
Expand Down
6 changes: 5 additions & 1 deletion server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.opensearch.action.ingest.GetPipelineRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.Strings;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestStatusToXContentListener;
Expand All @@ -48,6 +49,8 @@

public class RestGetPipelineAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetPipelineAction.class);

@Override
public List<Route> routes() {
return unmodifiableList(asList(new Route(GET, "/_ingest/pipeline"), new Route(GET, "/_ingest/pipeline/{id}")));
Expand All @@ -61,7 +64,8 @@ public String getName() {
@Override
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
GetPipelineRequest request = new GetPipelineRequest(Strings.splitStringByCommaToArray(restRequest.param("id")));
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
return channel -> client.admin().cluster().getPipeline(request, new RestStatusToXContentListener<>(channel));
}
}
6 changes: 5 additions & 1 deletion server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.bytes.BytesReference;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
Expand All @@ -49,6 +50,8 @@

public class RestPutPipelineAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutPipelineAction.class);

@Override
public List<Route> routes() {
return singletonList(new Route(PUT, "/_ingest/pipeline/{id}"));
Expand All @@ -63,7 +66,8 @@ public String getName() {
public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
Tuple<XContentType, BytesReference> sourceTuple = restRequest.contentOrSourceParam();
PutPipelineRequest request = new PutPipelineRequest(restRequest.param("id"), sourceTuple.v2(), sourceTuple.v1());
request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName());
request.timeout(restRequest.paramAsTime("timeout", request.timeout()));
return channel -> client.admin().cluster().putPipeline(request, new RestToXContentListener<>(channel));
}
Expand Down
59 changes: 59 additions & 0 deletions server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
import org.opensearch.rest.action.admin.indices.RestResizeHandler;
import org.opensearch.rest.action.admin.indices.RestRolloverIndexAction;
import org.opensearch.rest.action.admin.indices.RestUpdateSettingsAction;
import org.opensearch.rest.action.admin.cluster.RestDeleteStoredScriptAction;
import org.opensearch.rest.action.admin.cluster.RestGetStoredScriptAction;
import org.opensearch.rest.action.admin.cluster.RestPutStoredScriptAction;
import org.opensearch.rest.action.cat.RestAllocationAction;
import org.opensearch.rest.action.cat.RestRepositoriesAction;
import org.opensearch.rest.action.cat.RestThreadPoolAction;
Expand All @@ -54,6 +57,9 @@
import org.opensearch.rest.action.cat.RestPendingClusterTasksAction;
import org.opensearch.rest.action.cat.RestSegmentsAction;
import org.opensearch.rest.action.cat.RestSnapshotAction;
import org.opensearch.rest.action.ingest.RestDeletePipelineAction;
import org.opensearch.rest.action.ingest.RestGetPipelineAction;
import org.opensearch.rest.action.ingest.RestPutPipelineAction;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.TestThreadPool;
Expand Down Expand Up @@ -425,6 +431,59 @@ public void testImportDanglingIndex() {
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testDeletePipeline() {
FakeRestRequest request = new FakeRestRequest();
request.params().put("cluster_manager_timeout", "1h");
request.params().put("master_timeout", "3s");
request.params().put("id", "test");
RestDeletePipelineAction action = new RestDeletePipelineAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testGetPipeline() {
RestGetPipelineAction action = new RestGetPipelineAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testPutPipeline() {
FakeRestRequest request = getFakeRestRequestWithBody();
request.params().put("cluster_manager_timeout", "2m");
request.params().put("master_timeout", "3s");
request.params().put("id", "test");
RestPutPipelineAction action = new RestPutPipelineAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testDeleteStoredScript() {
RestDeleteStoredScriptAction action = new RestDeleteStoredScriptAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testGetStoredScript() {
RestGetStoredScriptAction action = new RestGetStoredScriptAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testPutStoredScript() {
RestPutStoredScriptAction action = new RestPutStoredScriptAction();
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE, "empty templates should no longer be used");
}

private MasterNodeRequest getMasterNodeRequest() {
return new MasterNodeRequest() {
@Override
Expand Down