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

Allow the deprecation warning of the REST API request parameter 'master_timeout' for the requests sent by High Level REST Client #2912

Closed
wants to merge 4 commits into from
+54 −15
Closed

Allow the deprecation warning of the REST API request parameter 'master_timeout' for the requests sent by High Level REST Client #2912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f5bcba
Skip adding deprecation warning message of the REST API request param…
Apr 15, 2022
e9f775d
Adjust format by spotlessApply task
Apr 15, 2022
b0d588e
Allow deprecation warning of master_timeout parameter in HLRC
Apr 15, 2022
e3d14d2
Remove all occurance of the master_timeour warning from the list
Apr 15, 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
29 changes: 26 additions & 3 deletions client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1756,7 +1756,7 @@ private <Req, Resp> Resp internalPerformRequest(
Set<Integer> ignores
) throws IOException {
Request req = requestConverter.apply(request);
req.setOptions(options);
req.setOptions(allowMasterTimeoutWarning(options));
Response response;
try {
response = client.performRequest(req);
Expand Down Expand Up @@ -1797,7 +1797,7 @@ protected final <Req extends Validatable, Resp> Optional<Resp> performRequestAnd
throw validationException.get();
}
Request req = requestConverter.apply(request);
req.setOptions(options);
req.setOptions(allowMasterTimeoutWarning(options));
Response response;
try {
response = client.performRequest(req);
Expand Down Expand Up @@ -1922,7 +1922,7 @@ private <Req, Resp> Cancellable internalPerformRequestAsync(
listener.onFailure(e);
return Cancellable.NO_OP;
}
req.setOptions(options);
req.setOptions(allowMasterTimeoutWarning(options));

ResponseListener responseListener = wrapResponseListener(responseConverter, listener, ignores);
return client.performRequestAsync(req, responseListener);
Expand Down Expand Up @@ -2178,4 +2178,27 @@ static List<NamedXContentRegistry.Entry> getProvidedNamedXContents() {
}
return entries;
}

protected static 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.";

protected RequestOptions allowMasterTimeoutWarning(RequestOptions options) {
WarningsHandler originalWarningsHandler = options.getWarningsHandler();
RequestOptions.Builder optionsBuilder = options.toBuilder();
optionsBuilder.setWarningsHandler(new WarningsHandler() {
@Override
public boolean warningsShouldFailRequest(List<String> warnings) {
if (originalWarningsHandler.warningsShouldFailRequest(Collections.singletonList(MASTER_TIMEOUT_DEPRECATED_MESSAGE))) {
warnings.removeAll(singleton(MASTER_TIMEOUT_DEPRECATED_MESSAGE));
}
return originalWarningsHandler.warningsShouldFailRequest(warnings);
}

@Override
public String toString() {
return originalWarningsHandler.toString();
}
});
return optionsBuilder.build();
}
}
23 changes: 23 additions & 0 deletions client/rest-high-level/src/test/java/org/opensearch/client/RestHighLevelClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.hamcrest.CoreMatchers.is;
import static org.opensearch.common.xcontent.XContentHelper.toXContent;
import static org.hamcrest.CoreMatchers.endsWith;
import static org.hamcrest.CoreMatchers.equalTo;
Expand Down Expand Up @@ -964,6 +965,28 @@ public void testApiNamingConventions() throws Exception {
assertThat("Some API are not supported but they should be: " + apiUnsupported, apiUnsupported.size(), equalTo(0));
}

public void testAllowMasterTimeoutWarning() throws IOException {
RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder();
builder.setWarningsHandler(WarningsHandler.STRICT);
builder.addHeader("test", "value");
HttpAsyncResponseConsumerFactory consumerFactory = new HttpAsyncResponseConsumerFactory.HeapBufferedResponseConsumerFactory(1);
builder.setHttpAsyncResponseConsumerFactory(consumerFactory);
RequestOptions originalOptions = builder.build();

RequestOptions modifiedOptions = restHighLevelClient.allowMasterTimeoutWarning(originalOptions);
// The modified WarningsHandler should only allow the specific deprecation warning message.
List<String> warnings = new ArrayList<>();
warnings.add(RestHighLevelClient.MASTER_TIMEOUT_DEPRECATED_MESSAGE);
warnings.add(RestHighLevelClient.MASTER_TIMEOUT_DEPRECATED_MESSAGE);
assertThat(modifiedOptions.getWarningsHandler().warningsShouldFailRequest(warnings), is(false));
warnings.add("another warning");
assertThat(modifiedOptions.getWarningsHandler().warningsShouldFailRequest(warnings), is(true));
assertThat(modifiedOptions.getWarningsHandler().toString(), is(WarningsHandler.STRICT.toString()));
// Other Options are not changed.
assertThat(modifiedOptions.getHeaders(), is(Collections.singletonList(new RequestOptions.ReqHeader("test", "value"))));
assertThat(modifiedOptions.getHttpAsyncResponseConsumerFactory(), is(consumerFactory));
}

private static void assertSyncMethod(Method method, String apiName, List<String> booleanReturnMethods) {
// A few methods return a boolean rather than a response object
if (apiName.equals("ping") || apiName.contains("exist") || booleanReturnMethods.contains(apiName)) {
Expand Down
9 changes: 5 additions & 4 deletions server/src/main/java/org/opensearch/rest/BaseRestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ protected Set<String> responseParams() {
return Collections.emptySet();
}

protected static 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.";
protected static final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";

/**
* 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.
Expand All @@ -218,10 +223,6 @@ public static void parseDeprecatedMasterTimeoutParameter(
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].";
if (request.hasParam("master_timeout")) {
logger.deprecate(logMsgKeyPrefix + "_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
if (request.hasParam("cluster_manager_timeout")) {
Expand Down
4 changes: 0 additions & 4 deletions server/src/main/java/org/opensearch/rest/action/admin/indices/RestGetMappingAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@
public class RestGetMappingAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetMappingAction.class);
private static 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.";
private static final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";

private final ThreadPool threadPool;

Expand Down
4 changes: 0 additions & 4 deletions server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ public class RestIndicesAction extends AbstractCatAction {

private static final DateFormatter STRICT_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_time");
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestIndicesAction.class);
private static 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.";
private static final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";

@Override
public List<Route> routes() {
Expand Down