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.

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

[2.x] Remove deprecation warning of using REST API request parameter 'master_timeout' #2920

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 6 additions & 15 deletions server/src/main/java/org/opensearch/rest/BaseRestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.CheckedConsumer;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.plugins.ActionPlugin;
Expand Down Expand Up @@ -203,27 +202,19 @@ protected Set<String> responseParams() {
return Collections.emptySet();
}

protected static final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";

Copy link
Collaborator Author

@tlfeng tlfeng Apr 15, 2022

Choose a reason for hiding this comment

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

Note: I moved the constant String outside the method to make it directly used in RestIndicesAction and RestAllocationAction classes in below change. In these 2 classes, the method parseDeprecatedMasterTimeoutParameter() can not fit the requirement, so need the error message used outside the BaseRestHandler, directly in the 2 classes.

/**
* Parse the deprecated request parameter 'master_timeout', and add deprecated log if the parameter is used.
* It also validates whether the two parameters 'master_timeout' and 'cluster_manager_timeout' are not assigned together.
* The method is temporarily added in 2.0 duing applying inclusive language. Remove the method along with MASTER_ROLE.
* Deprecation log is not emitted intentionally, because 'master_timeout' is remained in High-Level-REST-Client to keep compatibility with server 1.x.
* The method is temporarily added in 2.0 during applying inclusive language. Remove the method along with the parameter 'master_timeout'.
* @param mnr the action request
* @param request the REST request to handle
* @param logger the logger that logs deprecation notices
* @param logMsgKeyPrefix the key prefix of a deprecation message to avoid duplicate messages.
*/
public static void parseDeprecatedMasterTimeoutParameter(
MasterNodeRequest mnr,
RestRequest request,
DeprecationLogger logger,
String logMsgKeyPrefix
) {
final String MASTER_TIMEOUT_DEPRECATED_MESSAGE =
"Parameter [master_timeout] is deprecated and will be removed in 3.0. To support inclusive language, please use [cluster_manager_timeout] instead.";
final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";
public static void parseDeprecatedMasterTimeoutParameter(MasterNodeRequest mnr, RestRequest request) {
if (request.hasParam("master_timeout")) {
logger.deprecate(logMsgKeyPrefix + "_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
if (request.hasParam("cluster_manager_timeout")) {
throw new OpenSearchParseException(DUPLICATE_PARAMETER_ERROR_MESSAGE);
}
Copy link
Collaborator Author

@tlfeng tlfeng Apr 15, 2022

Choose a reason for hiding this comment

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

Note: the method arguments logger and logMsgKeyPrefix are removed since they are not used without deprecation warning.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
cleanupRepositoryRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", cleanupRepositoryRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(cleanupRepositoryRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(cleanupRepositoryRequest, request);
return channel -> client.admin().cluster().cleanupRepository(cleanupRepositoryRequest, new RestToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
XContentMapValues.nodeStringArrayValue(source.getOrDefault("indices", Collections.emptyList()))
);
cloneSnapshotRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", cloneSnapshotRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(cloneSnapshotRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(cloneSnapshotRequest, request);
cloneSnapshotRequest.indicesOptions(IndicesOptions.fromMap(source, cloneSnapshotRequest.indicesOptions()));
return channel -> client.admin().cluster().cloneSnapshot(cloneSnapshotRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
final boolean renderDefaults = request.paramAsBoolean("include_defaults", false);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request);
return channel -> client.admin().cluster().state(clusterStateRequest, new RestBuilderListener<ClusterStateResponse>(channel) {
@Override
public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder builder) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static ClusterHealthRequest fromRequest(final RestRequest request) {
clusterHealthRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterHealthRequest.indicesOptions()));
clusterHealthRequest.local(request.paramAsBoolean("local", clusterHealthRequest.local()));
clusterHealthRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterHealthRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterHealthRequest, request, deprecationLogger, "cluster_health");
parseDeprecatedMasterTimeoutParameter(clusterHealthRequest, request);
clusterHealthRequest.timeout(request.paramAsTime("timeout", clusterHealthRequest.timeout()));
String waitForStatus = request.param("wait_for_status");
if (waitForStatus != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public static ClusterRerouteRequest createRequest(RestRequest request) throws IO
clusterRerouteRequest.timeout(request.paramAsTime("timeout", clusterRerouteRequest.timeout()));
clusterRerouteRequest.setRetryFailed(request.paramAsBoolean("retry_failed", clusterRerouteRequest.isRetryFailed()));
clusterRerouteRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterRerouteRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterRerouteRequest, request, deprecationLogger, "cluster_reroute");
parseDeprecatedMasterTimeoutParameter(clusterRerouteRequest, request);
request.applyContentParser(parser -> PARSER.parse(parser, clusterRerouteRequest, null));
return clusterRerouteRequest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
clusterStateRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterStateRequest.indicesOptions()));
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request);
if (request.hasParam("wait_for_metadata_version")) {
clusterStateRequest.waitForMetadataVersion(request.paramAsLong("wait_for_metadata_version", 0));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
clusterUpdateSettingsRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", clusterUpdateSettingsRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(clusterUpdateSettingsRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(clusterUpdateSettingsRequest, request);
Map<String, Object> source;
try (XContentParser parser = request.contentParser()) {
source = parser.map();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
CreateSnapshotRequest createSnapshotRequest = createSnapshotRequest(request.param("repository"), request.param("snapshot"));
request.applyContentParser(p -> createSnapshotRequest.source(p.mapOrdered()));
createSnapshotRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", createSnapshotRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(createSnapshotRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(createSnapshotRequest, request);
createSnapshotRequest.waitForCompletion(request.paramAsBoolean("wait_for_completion", false));
return channel -> client.admin().cluster().createSnapshot(createSnapshotRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
deleteRepositoryRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", deleteRepositoryRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(deleteRepositoryRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(deleteRepositoryRequest, request);
return channel -> client.admin().cluster().deleteRepository(deleteRepositoryRequest, new RestToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
Strings.splitStringByCommaToArray(request.param("snapshot"))
);
deleteSnapshotRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", deleteSnapshotRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deleteSnapshotRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(deleteSnapshotRequest, request);
return channel -> client.admin().cluster().deleteSnapshot(deleteSnapshotRequest, new RestToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
deleteStoredScriptRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", deleteStoredScriptRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(deleteStoredScriptRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(deleteStoredScriptRequest, request);

return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
getRepositoriesRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", getRepositoriesRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(getRepositoriesRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(getRepositoriesRequest, request);
getRepositoriesRequest.local(request.paramAsBoolean("local", getRepositoriesRequest.local()));
settingsFilter.addFilterSettingParams(request);
return channel -> client.admin().cluster().getRepositories(getRepositoriesRequest, new RestToXContentListener<>(channel));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
getSnapshotsRequest.ignoreUnavailable(request.paramAsBoolean("ignore_unavailable", getSnapshotsRequest.ignoreUnavailable()));
getSnapshotsRequest.verbose(request.paramAsBoolean("verbose", getSnapshotsRequest.verbose()));
getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getSnapshotsRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(getSnapshotsRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(getSnapshotsRequest, request);
return channel -> client.admin().cluster().getSnapshots(getSnapshotsRequest, new RestToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient
String id = request.param("id");
GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id);
getRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(getRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(getRequest, request);
return channel -> client.admin().cluster().getStoredScript(getRequest, new RestStatusToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
pendingClusterTasksRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", pendingClusterTasksRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(pendingClusterTasksRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(pendingClusterTasksRequest, request);
pendingClusterTasksRequest.local(request.paramAsBoolean("local", pendingClusterTasksRequest.local()));
return channel -> client.admin().cluster().pendingClusterTasks(pendingClusterTasksRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
}
putRepositoryRequest.verify(request.paramAsBoolean("verify", true));
putRepositoryRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", putRepositoryRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(putRepositoryRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(putRepositoryRequest, request);
putRepositoryRequest.timeout(request.paramAsTime("timeout", putRepositoryRequest.timeout()));
return channel -> client.admin().cluster().putRepository(putRepositoryRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client

PutStoredScriptRequest putRequest = new PutStoredScriptRequest(id, context, content, request.getXContentType(), source);
putRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", putRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(putRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(putRequest, request);
putRequest.timeout(request.paramAsTime("timeout", putRequest.timeout()));
return channel -> client.admin().cluster().putStoredScript(putRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
restoreSnapshotRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", restoreSnapshotRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(restoreSnapshotRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(restoreSnapshotRequest, request);
restoreSnapshotRequest.waitForCompletion(request.paramAsBoolean("wait_for_completion", false));
request.applyContentParser(p -> restoreSnapshotRequest.source(p.mapOrdered()));
return channel -> client.admin().cluster().restoreSnapshot(restoreSnapshotRequest, new RestToXContentListener<>(channel));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
snapshotsStatusRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", snapshotsStatusRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(snapshotsStatusRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(snapshotsStatusRequest, request);
return channel -> client.admin().cluster().snapshotsStatus(snapshotsStatusRequest, new RestToXContentListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
verifyRepositoryRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", verifyRepositoryRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(verifyRepositoryRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(verifyRepositoryRequest, request);
verifyRepositoryRequest.timeout(request.paramAsTime("timeout", verifyRepositoryRequest.timeout()));
return channel -> client.admin().cluster().verifyRepository(verifyRepositoryRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient

deleteRequest.timeout(request.paramAsTime("timeout", deleteRequest.timeout()));
deleteRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", deleteRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deleteRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(deleteRequest, request);

return channel -> client.admin()
.cluster()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient

importRequest.timeout(request.paramAsTime("timeout", importRequest.timeout()));
importRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", importRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(importRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(importRequest, request);

return channel -> client.admin()
.cluster()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
Strings.splitStringByCommaToArray(request.param("index"))
);
addIndexBlockRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", addIndexBlockRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(addIndexBlockRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(addIndexBlockRequest, request);
addIndexBlockRequest.timeout(request.paramAsTime("timeout", addIndexBlockRequest.timeout()));
addIndexBlockRequest.indicesOptions(IndicesOptions.fromRequest(request, addIndexBlockRequest.indicesOptions()));
return channel -> client.admin().indices().addBlock(addIndexBlockRequest, new RestToXContentListener<>(channel));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public String getName() {
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
CloseIndexRequest closeIndexRequest = new CloseIndexRequest(Strings.splitStringByCommaToArray(request.param("index")));
closeIndexRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", closeIndexRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(closeIndexRequest, request, deprecationLogger, getName());
parseDeprecatedMasterTimeoutParameter(closeIndexRequest, request);
closeIndexRequest.timeout(request.paramAsTime("timeout", closeIndexRequest.timeout()));
closeIndexRequest.indicesOptions(IndicesOptions.fromRequest(request, closeIndexRequest.indicesOptions()));
String waitForActiveShards = request.param("wait_for_active_shards");
Expand Down
Loading