From 40c4e680e9ff9053b6f8ce718a31caca29c5e7d2 Mon Sep 17 00:00:00 2001 From: Pavan Yekbote Date: Thu, 9 Jan 2025 14:53:11 -0800 Subject: [PATCH] BWC (rag processor): add version control for newly added request params (#3125) * fix: handle bwc for new fields added in 2.13 Signed-off-by: Pavan Yekbote * fix: spotless changes Signed-off-by: Pavan Yekbote * fix: test cases with bwc Signed-off-by: Pavan Yekbote * fix: spotless apply Signed-off-by: Pavan Yekbote --------- Signed-off-by: Pavan Yekbote --- .../ext/GenerativeQAParameters.java | 32 +++++++++++++++---- .../ext/GenerativeQAParamExtBuilderTests.java | 17 +++++++--- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java index 7222b7369d..20b9f9107f 100644 --- a/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java +++ b/search-processors/src/main/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParameters.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.Objects; +import org.opensearch.Version; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -29,6 +30,7 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.ml.common.CommonValue; import com.google.common.base.Preconditions; @@ -81,6 +83,8 @@ public class GenerativeQAParameters implements Writeable, ToXContentObject { public static final int SIZE_NULL_VALUE = -1; + public static final Version MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS = CommonValue.VERSION_2_13_0; + @Setter @Getter private String conversationId; @@ -145,15 +149,23 @@ public GenerativeQAParameters( } public GenerativeQAParameters(StreamInput input) throws IOException { + Version version = input.getVersion(); this.conversationId = input.readOptionalString(); this.llmModel = input.readOptionalString(); this.llmQuestion = input.readString(); - this.systemPrompt = input.readOptionalString(); - this.userInstructions = input.readOptionalString(); + + if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) { + this.systemPrompt = input.readOptionalString(); + this.userInstructions = input.readOptionalString(); + } + this.contextSize = input.readInt(); this.interactionSize = input.readInt(); this.timeout = input.readInt(); - this.llmResponseField = input.readOptionalString(); + + if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) { + this.llmResponseField = input.readOptionalString(); + } } @Override @@ -201,17 +213,25 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params @Override public void writeTo(StreamOutput out) throws IOException { + Version version = out.getVersion(); out.writeOptionalString(conversationId); out.writeOptionalString(llmModel); Preconditions.checkNotNull(llmQuestion, "llm_question must not be null."); out.writeString(llmQuestion); - out.writeOptionalString(systemPrompt); - out.writeOptionalString(userInstructions); + + if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) { + out.writeOptionalString(systemPrompt); + out.writeOptionalString(userInstructions); + } + out.writeInt(contextSize); out.writeInt(interactionSize); out.writeInt(timeout); - out.writeOptionalString(llmResponseField); + + if (version.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS)) { + out.writeOptionalString(llmResponseField); + } } public static GenerativeQAParameters parse(XContentParser parser) throws IOException { diff --git a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java index 95374b14ea..3093a95372 100644 --- a/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java +++ b/search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParamExtBuilderTests.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.opensearch.core.xcontent.ToXContent.EMPTY_PARAMS; import java.io.EOFException; @@ -28,6 +29,7 @@ import java.util.Collections; import org.junit.Assert; +import org.opensearch.Version; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; @@ -105,10 +107,17 @@ public void testMiscMethods() throws IOException { assertNotEquals(builder1, builder2); assertNotEquals(builder1.hashCode(), builder2.hashCode()); - StreamOutput so = mock(StreamOutput.class); - builder1.writeTo(so); - verify(so, times(5)).writeOptionalString(any()); - verify(so, times(1)).writeString(any()); + StreamOutput so1 = mock(StreamOutput.class); + when(so1.getVersion()).thenReturn(GenerativeQAParameters.MINIMAL_SUPPORTED_VERSION_FOR_PROMPT_AND_INSTRUCTIONS); + builder1.writeTo(so1); + verify(so1, times(5)).writeOptionalString(any()); + verify(so1, times(1)).writeString(any()); + + StreamOutput so2 = mock(StreamOutput.class); + when(so2.getVersion()).thenReturn(Version.V_2_12_0); + builder1.writeTo(so2); + verify(so2, times(2)).writeOptionalString(any()); + verify(so1, times(1)).writeString(any()); } public void testParse() throws IOException {