From f73c9cf4696d3d66ab4a670b2ab99d44c74d420f Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 24 Nov 2023 13:30:20 +0800 Subject: [PATCH 1/3] move pplenabled to transportaction Signed-off-by: zane-neo --- .../org/opensearch/sql/plugin/SQLPlugin.java | 2 +- .../sql/plugin/rest/RestPPLQueryAction.java | 20 +------------------ .../transport/TransportPPLQueryAction.java | 20 ++++++++++++++++++- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 20a6834f90..f0689a0966 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -154,7 +154,7 @@ public List getRestHandlers( Metrics.getInstance().registerDefaultMetrics(); return Arrays.asList( - new RestPPLQueryAction(pluginSettings, settings), + new RestPPLQueryAction(), new RestSqlAction(settings, injector), new RestSqlStatsAction(settings, restController), new RestPPLStatsAction(settings, restController), diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java index 5bbb52fc3e..3358137fb3 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java @@ -49,16 +49,9 @@ public class RestPPLQueryAction extends BaseRestHandler { private static final Logger LOG = LogManager.getLogger(); - private final Supplier pplEnabled; - /** Constructor of RestPPLQueryAction. */ - public RestPPLQueryAction( - Settings pluginSettings, org.opensearch.common.settings.Settings clusterSettings) { + public RestPPLQueryAction() { super(); - this.pplEnabled = - () -> - MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings) - && (Boolean) pluginSettings.getSettingValue(Settings.Key.PPL_ENABLED); } private static boolean isClientError(Exception e) { @@ -104,17 +97,6 @@ protected Set responseParams() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient nodeClient) { - // TODO: need move to transport Action - if (!pplEnabled.get()) { - return channel -> - reportError( - channel, - new IllegalAccessException( - "Either plugins.ppl.enabled or rest.action.multi.allow_explicit_index setting is" - + " false"), - BAD_REQUEST); - } - TransportPPLQueryRequest transportPPLQueryRequest = new TransportPPLQueryRequest(PPLQueryRequestFactory.getPPLRequest(request)); diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index fde9e24f75..047a556561 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -5,10 +5,13 @@ package org.opensearch.sql.plugin.transport; +import static org.opensearch.rest.BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX; import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; import java.util.Locale; import java.util.Optional; +import java.util.function.Supplier; + import org.opensearch.action.ActionRequest; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; @@ -19,6 +22,7 @@ import org.opensearch.common.inject.ModulesBuilder; import org.opensearch.core.action.ActionListener; import org.opensearch.sql.common.response.ResponseListener; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.common.utils.QueryContext; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.datasources.service.DataSourceServiceImpl; @@ -47,6 +51,8 @@ public class TransportPPLQueryAction private final Injector injector; + private final Supplier pplEnabled; + /** Constructor of TransportPPLQueryAction. */ @Inject public TransportPPLQueryAction( @@ -54,7 +60,9 @@ public TransportPPLQueryAction( ActionFilters actionFilters, NodeClient client, ClusterService clusterService, - DataSourceServiceImpl dataSourceService) { + DataSourceServiceImpl dataSourceService, + Settings pluginSettings, + org.opensearch.common.settings.Settings clusterSettings) { super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new); ModulesBuilder modules = new ModulesBuilder(); @@ -67,6 +75,10 @@ public TransportPPLQueryAction( b.bind(DataSourceService.class).toInstance(dataSourceService); }); this.injector = modules.createInjector(); + this.pplEnabled = + () -> + MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings) + && (Boolean) pluginSettings.getSettingValue(Settings.Key.PPL_ENABLED); } /** @@ -76,6 +88,12 @@ public TransportPPLQueryAction( @Override protected void doExecute( Task task, ActionRequest request, ActionListener listener) { + if (!pplEnabled.get()) { + listener.onFailure(new IllegalAccessException( + "Either plugins.ppl.enabled or rest.action.multi.allow_explicit_index setting is" + + " false")); + return; + } Metrics.getInstance().getNumericalMetric(MetricName.PPL_REQ_TOTAL).increment(); Metrics.getInstance().getNumericalMetric(MetricName.PPL_REQ_COUNT_TOTAL).increment(); From 35f0e677bcdd46cb28e491705b0d4261fc040373 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Wed, 6 Dec 2023 09:50:40 +0800 Subject: [PATCH 2/3] Fix IT failure and format code Signed-off-by: zane-neo --- .../sql/plugin/rest/RestPPLQueryAction.java | 2 -- .../plugin/transport/TransportPPLQueryAction.java | 14 ++++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java index 3358137fb3..d35962be91 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestPPLQueryAction.java @@ -15,7 +15,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.Supplier; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchSecurityException; @@ -28,7 +27,6 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.sql.common.antlr.SyntaxCheckException; -import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.datasources.exceptions.DataSourceClientException; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.QueryEngineException; diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index 047a556561..76283ac63a 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -11,7 +11,6 @@ import java.util.Locale; import java.util.Optional; import java.util.function.Supplier; - import org.opensearch.action.ActionRequest; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; @@ -61,7 +60,6 @@ public TransportPPLQueryAction( NodeClient client, ClusterService clusterService, DataSourceServiceImpl dataSourceService, - Settings pluginSettings, org.opensearch.common.settings.Settings clusterSettings) { super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new); @@ -78,7 +76,10 @@ public TransportPPLQueryAction( this.pplEnabled = () -> MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings) - && (Boolean) pluginSettings.getSettingValue(Settings.Key.PPL_ENABLED); + && (Boolean) + injector + .getInstance(org.opensearch.sql.common.setting.Settings.class) + .getSettingValue(Settings.Key.PPL_ENABLED); } /** @@ -89,9 +90,10 @@ public TransportPPLQueryAction( protected void doExecute( Task task, ActionRequest request, ActionListener listener) { if (!pplEnabled.get()) { - listener.onFailure(new IllegalAccessException( - "Either plugins.ppl.enabled or rest.action.multi.allow_explicit_index setting is" - + " false")); + listener.onFailure( + new IllegalAccessException( + "Either plugins.ppl.enabled or rest.action.multi.allow_explicit_index setting is" + + " false")); return; } Metrics.getInstance().getNumericalMetric(MetricName.PPL_REQ_TOTAL).increment(); From 3162b81647264224dc534dabb02ace7f3d316a00 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 8 Dec 2023 22:17:55 +0800 Subject: [PATCH 3/3] Fix failure IT Signed-off-by: zane-neo --- docs/user/ppl/admin/settings.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/user/ppl/admin/settings.rst b/docs/user/ppl/admin/settings.rst index 1a13d66d4f..ad56408693 100644 --- a/docs/user/ppl/admin/settings.rst +++ b/docs/user/ppl/admin/settings.rst @@ -63,6 +63,7 @@ PPL query:: sh$ curl -sS -H 'Content-Type: application/json' \ ... -X POST localhost:9200/_plugins/_ppl \ + ... -d '{"query": "source=my_prometheus"}' { "error": { "reason": "Invalid Query",