From 9289d179fa54bac6d8eb9363c7856734e35e7762 Mon Sep 17 00:00:00 2001 From: Eric Date: Wed, 8 Nov 2023 21:24:29 +0000 Subject: [PATCH] Revert "Block settings in sql query settings API and add more unit tests (#2407) (#2412)" This reverts commit 3024737fe613af166c7a3ef18a6fb054a4f726d8. Signed-off-by: Eric --- docs/user/admin/settings.rst | 6 +++--- .../plugin/rest/RestQuerySettingsAction.java | 9 -------- .../client/EmrServerlessClientImplTest.java | 21 +------------------ .../sql/spark/constants/TestConstants.java | 3 --- 4 files changed, 4 insertions(+), 35 deletions(-) diff --git a/docs/user/admin/settings.rst b/docs/user/admin/settings.rst index 7e175ae719..f3e8070a23 100644 --- a/docs/user/admin/settings.rst +++ b/docs/user/admin/settings.rst @@ -328,7 +328,7 @@ You can update the setting with a new value like this. SQL query:: - sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings \ + sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_query/settings \ ... -d '{"transient":{"plugins.query.executionengine.spark.session.limit":200}}' { "acknowledged": true, @@ -365,7 +365,7 @@ You can update the setting with a new value like this. SQL query:: - sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings \ + sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_query/settings \ ... -d '{"transient":{"plugins.query.executionengine.spark.refresh_job.limit":200}}' { "acknowledged": true, @@ -402,7 +402,7 @@ You can update the setting with a new value like this. SQL query:: - sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_cluster/settings \ + sh$ curl -sS -H 'Content-Type: application/json' -X PUT localhost:9200/_plugins/_query/settings \ ... -d '{"transient":{"plugins.query.datasources.limit":25}}' { "acknowledged": true, diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java index f9080051b4..885c953c17 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestQuerySettingsAction.java @@ -39,8 +39,6 @@ public class RestQuerySettingsAction extends BaseRestHandler { private static final String LEGACY_SQL_SETTINGS_PREFIX = "opendistro.sql."; private static final String LEGACY_PPL_SETTINGS_PREFIX = "opendistro.ppl."; private static final String LEGACY_COMMON_SETTINGS_PREFIX = "opendistro.query."; - private static final String EXECUTION_ENGINE_SETTINGS_PREFIX = "plugins.query.executionengine"; - public static final String DATASOURCES_SETTINGS_PREFIX = "plugins.query.datasources"; private static final List SETTINGS_PREFIX = ImmutableList.of( SQL_SETTINGS_PREFIX, @@ -50,9 +48,6 @@ public class RestQuerySettingsAction extends BaseRestHandler { LEGACY_PPL_SETTINGS_PREFIX, LEGACY_COMMON_SETTINGS_PREFIX); - private static final List DENY_LIST_SETTINGS_PREFIX = - ImmutableList.of(EXECUTION_ENGINE_SETTINGS_PREFIX, DATASOURCES_SETTINGS_PREFIX); - public static final String SETTINGS_API_ENDPOINT = "/_plugins/_query/settings"; public static final String LEGACY_SQL_SETTINGS_API_ENDPOINT = "/_opendistro/_sql/settings"; @@ -138,10 +133,6 @@ private Settings getAndFilterSettings(Map source) { } return true; }); - // Applying DenyList Filter. - settingsBuilder - .keys() - .removeIf(key -> DENY_LIST_SETTINGS_PREFIX.stream().anyMatch(key::startsWith)); return settingsBuilder.build(); } catch (IOException e) { throw new OpenSearchGenerationException("Failed to generate [" + source + "]", e); diff --git a/spark/src/test/java/org/opensearch/sql/spark/client/EmrServerlessClientImplTest.java b/spark/src/test/java/org/opensearch/sql/spark/client/EmrServerlessClientImplTest.java index 8129c3b0e0..319e887171 100644 --- a/spark/src/test/java/org/opensearch/sql/spark/client/EmrServerlessClientImplTest.java +++ b/spark/src/test/java/org/opensearch/sql/spark/client/EmrServerlessClientImplTest.java @@ -8,15 +8,11 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.opensearch.sql.spark.constants.TestConstants.DEFAULT_RESULT_INDEX; import static org.opensearch.sql.spark.constants.TestConstants.EMRS_APPLICATION_ID; import static org.opensearch.sql.spark.constants.TestConstants.EMRS_EXECUTION_ROLE; import static org.opensearch.sql.spark.constants.TestConstants.EMRS_JOB_NAME; import static org.opensearch.sql.spark.constants.TestConstants.EMR_JOB_ID; -import static org.opensearch.sql.spark.constants.TestConstants.ENTRY_POINT_START_JAR; import static org.opensearch.sql.spark.constants.TestConstants.QUERY; import static org.opensearch.sql.spark.constants.TestConstants.SPARK_SUBMIT_PARAMETERS; @@ -24,17 +20,13 @@ import com.amazonaws.services.emrserverless.model.CancelJobRunResult; import com.amazonaws.services.emrserverless.model.GetJobRunResult; import com.amazonaws.services.emrserverless.model.JobRun; -import com.amazonaws.services.emrserverless.model.StartJobRunRequest; import com.amazonaws.services.emrserverless.model.StartJobRunResult; import com.amazonaws.services.emrserverless.model.ValidationException; import java.util.HashMap; -import java.util.List; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.sql.common.setting.Settings; @@ -48,8 +40,6 @@ public class EmrServerlessClientImplTest { @Mock private OpenSearchSettings settings; - @Captor private ArgumentCaptor startJobRunRequestArgumentCaptor; - @BeforeEach public void setUp() { doReturn(emptyList()).when(settings).getSettings(); @@ -74,16 +64,7 @@ void testStartJobRun() { SPARK_SUBMIT_PARAMETERS, new HashMap<>(), false, - DEFAULT_RESULT_INDEX)); - verify(emrServerless, times(1)).startJobRun(startJobRunRequestArgumentCaptor.capture()); - StartJobRunRequest startJobRunRequest = startJobRunRequestArgumentCaptor.getValue(); - Assertions.assertEquals(EMRS_APPLICATION_ID, startJobRunRequest.getApplicationId()); - Assertions.assertEquals(EMRS_EXECUTION_ROLE, startJobRunRequest.getExecutionRoleArn()); - Assertions.assertEquals( - ENTRY_POINT_START_JAR, startJobRunRequest.getJobDriver().getSparkSubmit().getEntryPoint()); - Assertions.assertEquals( - List.of(QUERY, DEFAULT_RESULT_INDEX), - startJobRunRequest.getJobDriver().getSparkSubmit().getEntryPointArguments()); + null)); } @Test diff --git a/spark/src/test/java/org/opensearch/sql/spark/constants/TestConstants.java b/spark/src/test/java/org/opensearch/sql/spark/constants/TestConstants.java index cc13061323..3a0d8fc56d 100644 --- a/spark/src/test/java/org/opensearch/sql/spark/constants/TestConstants.java +++ b/spark/src/test/java/org/opensearch/sql/spark/constants/TestConstants.java @@ -18,7 +18,4 @@ public class TestConstants { public static final String TEST_CLUSTER_NAME = "TEST_CLUSTER"; public static final String MOCK_SESSION_ID = "s-0123456"; public static final String MOCK_STATEMENT_ID = "st-0123456"; - public static final String ENTRY_POINT_START_JAR = - "file:///home/hadoop/.ivy2/jars/org.opensearch_opensearch-spark-sql-application_2.12-0.1.0-SNAPSHOT.jar"; - public static final String DEFAULT_RESULT_INDEX = "query_execution_result_ds1"; }