From 574986809952b420621f3bfe3eb505c2d093f23f Mon Sep 17 00:00:00 2001 From: HenryL27 Date: Tue, 13 Feb 2024 14:27:41 -0800 Subject: [PATCH] Enable conversation memory feature flags (#2095) * Enable conversation memory feature flags Signed-off-by: HenryL27 * amend feature flag tests Signed-off-by: HenryL27 * remove accidentally committed files Signed-off-by: HenryL27 * apply spotless Signed-off-by: HenryL27 * fix final feature disablement test. whoops Signed-off-by: HenryL27 * move common feature disablement code to util class Signed-off-by: HenryL27 --------- Signed-off-by: HenryL27 (cherry picked from commit 850c7485f0da3220ae20da0b455b667fa773380d) --- .../ConversationalIndexConstants.java | 2 +- .../opensearch/ml/memory/MemoryTestUtil.java | 41 +++++++++++++++++++ ...reateConversationTransportActionTests.java | 4 +- ...CreateInteractionTransportActionTests.java | 4 +- ...eleteConversationTransportActionTests.java | 4 +- .../GetConversationTransportActionTests.java | 4 +- .../GetConversationsTransportActionTests.java | 4 +- .../GetInteractionTransportActionTests.java | 4 +- .../GetInteractionsTransportActionTests.java | 4 +- ...archConversationsTransportActionTests.java | 4 +- ...archInteractionsTransportActionsTests.java | 4 +- .../GenerativeQAProcessorConstants.java | 2 +- .../GenerativeQARequestProcessorTests.java | 9 ---- 13 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 memory/src/test/java/org/opensearch/ml/memory/MemoryTestUtil.java diff --git a/common/src/main/java/org/opensearch/ml/common/conversation/ConversationalIndexConstants.java b/common/src/main/java/org/opensearch/ml/common/conversation/ConversationalIndexConstants.java index 23cad0064d..a2ecced3e3 100644 --- a/common/src/main/java/org/opensearch/ml/common/conversation/ConversationalIndexConstants.java +++ b/common/src/main/java/org/opensearch/ml/common/conversation/ConversationalIndexConstants.java @@ -121,5 +121,5 @@ public class ConversationalIndexConstants { /** Feature Flag setting for conversational memory */ public static final Setting ML_COMMONS_MEMORY_FEATURE_ENABLED = Setting - .boolSetting("plugins.ml_commons.memory_feature_enabled", false, Setting.Property.NodeScope, Setting.Property.Dynamic); + .boolSetting("plugins.ml_commons.memory_feature_enabled", true, Setting.Property.NodeScope, Setting.Property.Dynamic); } \ No newline at end of file diff --git a/memory/src/test/java/org/opensearch/ml/memory/MemoryTestUtil.java b/memory/src/test/java/org/opensearch/ml/memory/MemoryTestUtil.java new file mode 100644 index 0000000000..96cdf8df3c --- /dev/null +++ b/memory/src/test/java/org/opensearch/ml/memory/MemoryTestUtil.java @@ -0,0 +1,41 @@ +/* + * Copyright 2024 Aryn + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.opensearch.ml.memory; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Set; + +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.ml.common.conversation.ConversationalIndexConstants; + +public class MemoryTestUtil { + + public static ClusterService clusterServiceWithMemoryFeatureDisabled() { + ClusterService mockClusterService = mock(ClusterService.class); + Settings settings = Settings.builder().put(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED.getKey(), false).build(); + when(mockClusterService.getSettings()).thenReturn(settings); + when(mockClusterService.getClusterSettings()) + .thenReturn(new ClusterSettings(settings, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + return mockClusterService; + } + +} diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportActionTests.java index c2e4b16e65..575e443461 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportActionTests.java @@ -41,6 +41,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -154,8 +155,7 @@ public void testDoExecuteFails_thenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new CreateConversationTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportActionTests.java index 22ec5ce386..182f4ed79e 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportActionTests.java @@ -45,6 +45,7 @@ import org.opensearch.core.index.shard.ShardId; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -227,8 +228,7 @@ public void testDoExecuteFails_thenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new CreateInteractionTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java index 984b9a2fbf..6fd3ecdecc 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportActionTests.java @@ -40,6 +40,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -138,8 +139,7 @@ public void testdoExecuteFails_thenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new DeleteConversationTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationTransportActionTests.java index 97ff87f63b..bb88a5c1ed 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationTransportActionTests.java @@ -43,6 +43,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationMeta; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -138,8 +139,7 @@ public void testHandlerThrows_ThenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new GetConversationTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportActionTests.java index 130d39c5cb..f0cd87f0e4 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportActionTests.java @@ -44,6 +44,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationMeta; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -193,8 +194,7 @@ public void testdoExecuteFails_thenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new GetConversationsTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionTransportActionTests.java index fa7fd52918..8f2878e9e7 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionTransportActionTests.java @@ -44,6 +44,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; import org.opensearch.ml.common.conversation.Interaction; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -147,8 +148,7 @@ public void testHandlerThrows_ThenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new GetInteractionTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportActionTests.java index 7b4b62df15..81f20277f4 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportActionTests.java @@ -45,6 +45,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; import org.opensearch.ml.common.conversation.Interaction; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -185,8 +186,7 @@ public void testDoExecuteFails_thenFail() { } public void testFeatureDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new GetInteractionsTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchConversationsTransportActionTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchConversationsTransportActionTests.java index 7ec9d8c042..2a43ed0176 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchConversationsTransportActionTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchConversationsTransportActionTests.java @@ -43,6 +43,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -109,8 +110,7 @@ public void testEnabled_ThenSucceed() { } public void testDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new SearchConversationsTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchInteractionsTransportActionsTests.java b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchInteractionsTransportActionsTests.java index abe5204c65..db6c2c20a3 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchInteractionsTransportActionsTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/action/conversation/SearchInteractionsTransportActionsTests.java @@ -42,6 +42,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; +import org.opensearch.ml.memory.MemoryTestUtil; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.ThreadPool; @@ -109,8 +110,7 @@ public void testFeatureEnabled_ThenSucceed() { } public void testDisabled_ThenFail() { - when(this.clusterService.getSettings()).thenReturn(Settings.EMPTY); - when(this.clusterService.getClusterSettings()).thenReturn(new ClusterSettings(Settings.EMPTY, Set.of(ConversationalIndexConstants.ML_COMMONS_MEMORY_FEATURE_ENABLED))); + clusterService = MemoryTestUtil.clusterServiceWithMemoryFeatureDisabled(); this.action = spy(new SearchInteractionsTransportAction(transportService, actionFilters, cmHandler, client, clusterService)); action.doExecute(null, request, actionListener); diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java index 957e01b302..ff71cadba2 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQAProcessorConstants.java @@ -40,7 +40,7 @@ public class GenerativeQAProcessorConstants { public static final String CONFIG_NAME_USER_INSTRUCTIONS = "user_instructions"; public static final Setting RAG_PIPELINE_FEATURE_ENABLED = Setting - .boolSetting("plugins.ml_commons.rag_pipeline_feature_enabled", false, Setting.Property.NodeScope, Setting.Property.Dynamic); + .boolSetting("plugins.ml_commons.rag_pipeline_feature_enabled", true, Setting.Property.NodeScope, Setting.Property.Dynamic); public static final String FEATURE_NOT_ENABLED_ERROR_MSG = RAG_PIPELINE_FEATURE_ENABLED.getKey() + " is not enabled."; } diff --git a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQARequestProcessorTests.java b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQARequestProcessorTests.java index 69301196ac..23da8758bf 100644 --- a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQARequestProcessorTests.java +++ b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeQARequestProcessorTests.java @@ -59,15 +59,6 @@ public void testGetType() { assertEquals(GenerativeQAProcessorConstants.REQUEST_PROCESSOR_TYPE, processor.getType()); } - public void testProcessorFactoryFeatureFlagDisabled() throws Exception { - - exceptionRule.expect(MLException.class); - exceptionRule.expectMessage(GenerativeQAProcessorConstants.FEATURE_NOT_ENABLED_ERROR_MSG); - Map config = new HashMap<>(); - config.put("model_id", "foo"); - Processor processor = new GenerativeQARequestProcessor.Factory(() -> false).create(null, "tag", "desc", true, config, null); - } - // Only to be used for the following test case. private boolean featureFlag001 = false;