Skip to content

Commit fce2020

Browse files
dhrubo-osdhrubo-os
authored
Merge branch 'main' into main-fix-validate-modelinput
2 parents 1ff7f90 + 6d33e91 commit fce2020

File tree

23 files changed

+761
-49
lines changed

23 files changed

+761
-49
lines changed

‎.github/workflows/CI-workflow.yml‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ name: Build and Test ml-commons
22
# This workflow is triggered on pull requests and push to any branches
33
on:
44
push:
5-
branches-ignore:
6-
- 'backport/**'
7-
- 'create-pull-request/**'
8-
- 'dependabot/**'
5+
branches:
6+
- 'main'
7+
- '[0-9]+.[0-9]+'
8+
- '[0-9]+.x'
99
pull_request_target:
1010
types: [opened, synchronize, reopened]
1111

@@ -42,7 +42,7 @@ jobs:
4242
needs: [Get-Require-Approval, Get-CI-Image-Tag, spotless]
4343
strategy:
4444
matrix:
45-
java: [21, 24]
45+
java: [21, 25]
4646

4747
name: Build and Test MLCommons Plugin on linux
4848
if: github.repository == 'opensearch-project/ml-commons'
@@ -68,11 +68,12 @@ jobs:
6868
with:
6969
role-to-assume: ${{ secrets.ML_ROLE }}
7070
aws-region: us-west-2
71+
7172
- name: Checkout MLCommons
7273
uses: actions/checkout@v4
7374
with:
7475
ref: ${{ github.event.pull_request.head.sha }}
75-
76+
7677
- name: Build and Run Tests
7778
id: step-build-test-linux
7879
run: |
@@ -107,7 +108,7 @@ jobs:
107108
needs: [Get-Require-Approval, Build-ml-linux, spotless]
108109
strategy:
109110
matrix:
110-
java: [21, 24]
111+
java: [21, 25]
111112

112113
name: Test MLCommons Plugin on linux docker
113114
if: github.repository == 'opensearch-project/ml-commons'
@@ -203,7 +204,7 @@ jobs:
203204
Build-ml-windows:
204205
strategy:
205206
matrix:
206-
java: [21, 24]
207+
java: [21, 25]
207208
name: Build and Test MLCommons Plugin on Windows
208209
if: github.repository == 'opensearch-project/ml-commons'
209210
needs: [Get-Require-Approval, spotless]

‎build.gradle‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,10 @@ allprojects {
7575
apply from: "$rootDir/build-tools/repositories.gradle"
7676

7777
plugins.withId('java') {
78-
sourceCompatibility = JavaVersion.VERSION_21;
79-
targetCompatibility = JavaVersion.VERSION_21;
78+
java {
79+
sourceCompatibility = JavaVersion.VERSION_21
80+
targetCompatibility = JavaVersion.VERSION_21
81+
}
8082
}
8183

8284
plugins.withId('jacoco') {
@@ -88,8 +90,10 @@ allprojects {
8890
}
8991

9092
subprojects {
91-
configurations {
92-
testImplementation.extendsFrom compileOnly
93+
plugins.withId('java') {
94+
configurations {
95+
testImplementation.extendsFrom compileOnly
96+
}
9397
}
9498

9599
configurations.all {

‎common/src/main/java/org/opensearch/ml/common/CommonValue.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public class CommonValue {
9797
public static final Version VERSION_3_1_0 = Version.fromString("3.1.0");
9898
public static final Version VERSION_3_2_0 = Version.fromString("3.2.0");
9999
public static final Version VERSION_3_3_0 = Version.fromString("3.3.0");
100+
public static final Version VERSION_3_4_0 = Version.fromString("3.4.0");
100101

101102
// Connector Constants
102103
public static final String NAME_FIELD = "name";

‎common/src/main/java/org/opensearch/ml/common/agent/MLToolSpec.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
147147
if (type != null) {
148148
builder.field(TOOL_TYPE_FIELD, type);
149149
}
150-
if (name != null) {
150+
if (name != null && !name.isEmpty()) {
151151
builder.field(TOOL_NAME_FIELD, name);
152152
}
153-
if (description != null) {
153+
if (description != null && !description.isEmpty()) {
154154
builder.field(DESCRIPTION_FIELD, description);
155155
}
156156
if (attributes != null && !attributes.isEmpty()) {

‎common/src/main/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInput.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
99
import static org.opensearch.ml.common.CommonValue.TENANT_ID_FIELD;
1010
import static org.opensearch.ml.common.CommonValue.VERSION_2_19_0;
11+
import static org.opensearch.ml.common.CommonValue.VERSION_3_4_0;
1112
import static org.opensearch.ml.common.utils.StringUtils.getParameterMap;
1213

1314
import java.io.IOException;
@@ -51,12 +52,14 @@ public class MLAgentUpdateInput implements ToXContentObject, Writeable {
5152
public static final String MEMORY_TYPE_FIELD = "type";
5253
public static final String MEMORY_SESSION_ID_FIELD = "session_id";
5354
public static final String MEMORY_WINDOW_SIZE_FIELD = "window_size";
55+
public static final String TYPE_FIELD = "type";
5456
public static final String APP_TYPE_FIELD = "app_type";
5557
public static final String LAST_UPDATED_TIME_FIELD = "last_updated_time";
5658

5759
@Getter
5860
private String agentId;
5961
private String name;
62+
private String type;
6063
private String description;
6164
private String llmModelId;
6265
private Map<String, String> llmParameters;
@@ -73,6 +76,7 @@ public class MLAgentUpdateInput implements ToXContentObject, Writeable {
7376
public MLAgentUpdateInput(
7477
String agentId,
7578
String name,
79+
String type,
7680
String description,
7781
String llmModelId,
7882
Map<String, String> llmParameters,
@@ -87,6 +91,7 @@ public MLAgentUpdateInput(
8791
) {
8892
this.agentId = agentId;
8993
this.name = name;
94+
this.type = type;
9095
this.description = description;
9196
this.llmModelId = llmModelId;
9297
this.llmParameters = llmParameters;
@@ -105,6 +110,7 @@ public MLAgentUpdateInput(StreamInput in) throws IOException {
105110
Version streamInputVersion = in.getVersion();
106111
agentId = in.readString();
107112
name = in.readOptionalString();
113+
type = streamInputVersion.onOrAfter(VERSION_3_4_0) ? in.readOptionalString() : null;
108114
description = in.readOptionalString();
109115
llmModelId = in.readOptionalString();
110116
if (in.readBoolean()) {
@@ -135,6 +141,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
135141
if (name != null) {
136142
builder.field(AGENT_NAME_FIELD, name);
137143
}
144+
if (type != null) {
145+
builder.field(TYPE_FIELD, type);
146+
}
138147
if (description != null) {
139148
builder.field(DESCRIPTION_FIELD, description);
140149
}
@@ -185,6 +194,9 @@ public void writeTo(StreamOutput out) throws IOException {
185194
Version streamOutputVersion = out.getVersion();
186195
out.writeString(agentId);
187196
out.writeOptionalString(name);
197+
if (streamOutputVersion.onOrAfter(VERSION_3_4_0)) {
198+
out.writeOptionalString(type);
199+
}
188200
out.writeOptionalString(description);
189201
out.writeOptionalString(llmModelId);
190202
if (llmParameters != null && !llmParameters.isEmpty()) {
@@ -221,6 +233,7 @@ public void writeTo(StreamOutput out) throws IOException {
221233
public static MLAgentUpdateInput parse(XContentParser parser) throws IOException {
222234
String agentId = null;
223235
String name = null;
236+
String type = null;
224237
String description = null;
225238
String llmModelId = null;
226239
Map<String, String> llmParameters = null;
@@ -244,6 +257,9 @@ public static MLAgentUpdateInput parse(XContentParser parser) throws IOException
244257
case AGENT_NAME_FIELD:
245258
name = parser.text();
246259
break;
260+
case TYPE_FIELD:
261+
type = parser.text();
262+
break;
247263
case DESCRIPTION_FIELD:
248264
description = parser.text();
249265
break;
@@ -314,6 +330,7 @@ public static MLAgentUpdateInput parse(XContentParser parser) throws IOException
314330
return new MLAgentUpdateInput(
315331
agentId,
316332
name,
333+
type,
317334
description,
318335
llmModelId,
319336
llmParameters,
@@ -329,6 +346,9 @@ public static MLAgentUpdateInput parse(XContentParser parser) throws IOException
329346
}
330347

331348
public MLAgent toMLAgent(MLAgent originalAgent) {
349+
if (type != null && !type.equals(originalAgent.getType())) {
350+
throw new IllegalArgumentException("Agent type cannot be updated");
351+
}
332352
LLMSpec finalLlm;
333353
if (llmModelId == null && (llmParameters == null || llmParameters.isEmpty())) {
334354
finalLlm = originalAgent.getLlm();

‎common/src/test/java/org/opensearch/ml/common/agent/MLToolSpecTest.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,84 @@ public void toXContentNullConfigMap() throws IOException {
125125
);
126126
}
127127

128+
@Test
129+
public void toXContentEmptyName() throws IOException {
130+
MLToolSpec specWithEmptyName = new MLToolSpec(
131+
"test_type",
132+
"", // Empty name
133+
"test_desc",
134+
Map.of("test_key", "test_value"),
135+
Collections.emptyMap(),
136+
false,
137+
Map.of("config_key", "config_value"),
138+
null,
139+
null
140+
);
141+
142+
XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
143+
specWithEmptyName.toXContent(builder, ToXContent.EMPTY_PARAMS);
144+
String content = TestHelper.xContentBuilderToString(builder);
145+
146+
Assert.assertFalse(content.contains("name"));
147+
Assert
148+
.assertEquals(
149+
"{\"type\":\"test_type\",\"description\":\"test_desc\",\"parameters\":{\"test_key\":\"test_value\"},\"include_output_in_agent_response\":false,\"config\":{\"config_key\":\"config_value\"}}",
150+
content
151+
);
152+
}
153+
154+
@Test
155+
public void toXContentEmptyDescription() throws IOException {
156+
MLToolSpec specWithEmptyDesc = new MLToolSpec(
157+
"test_type",
158+
"test_name",
159+
"", // Empty description
160+
Map.of("test_key", "test_value"),
161+
Collections.emptyMap(),
162+
false,
163+
Map.of("config_key", "config_value"),
164+
null,
165+
null
166+
);
167+
168+
XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
169+
specWithEmptyDesc.toXContent(builder, ToXContent.EMPTY_PARAMS);
170+
String content = TestHelper.xContentBuilderToString(builder);
171+
172+
Assert.assertFalse(content.contains("description"));
173+
Assert
174+
.assertEquals(
175+
"{\"type\":\"test_type\",\"name\":\"test_name\",\"parameters\":{\"test_key\":\"test_value\"},\"include_output_in_agent_response\":false,\"config\":{\"config_key\":\"config_value\"}}",
176+
content
177+
);
178+
}
179+
180+
@Test
181+
public void toXContentEmptyParameters() throws IOException {
182+
MLToolSpec specWithEmptyParams = new MLToolSpec(
183+
"test_type",
184+
"test_name",
185+
"test_desc",
186+
Collections.emptyMap(), // Empty parameters
187+
Collections.emptyMap(),
188+
false,
189+
Map.of("config_key", "config_value"),
190+
null,
191+
null
192+
);
193+
194+
XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
195+
specWithEmptyParams.toXContent(builder, ToXContent.EMPTY_PARAMS);
196+
String content = TestHelper.xContentBuilderToString(builder);
197+
198+
Assert.assertFalse(content.contains("parameters"));
199+
Assert
200+
.assertEquals(
201+
"{\"type\":\"test_type\",\"name\":\"test_name\",\"description\":\"test_desc\",\"include_output_in_agent_response\":false,\"config\":{\"config_key\":\"config_value\"}}",
202+
content
203+
);
204+
}
205+
128206
@Test
129207
public void parse() throws IOException {
130208
String jsonStr =

‎common/src/test/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInputTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ public void testParseWithAllFields() throws Exception {
392392
{
393393
"agent_id": "test-agent-id",
394394
"name": "test-agent",
395+
"type": "flow",
395396
"description": "test description",
396397
"llm": {
397398
"model_id": "test-model-id",
@@ -423,6 +424,7 @@ public void testParseWithAllFields() throws Exception {
423424
""";
424425
testParseFromJsonString(inputStr, parsedInput -> {
425426
assertEquals("test-agent", parsedInput.getName());
427+
assertEquals("flow", parsedInput.getType());
426428
assertEquals("test description", parsedInput.getDescription());
427429
assertEquals("test-model-id", parsedInput.getLlmModelId());
428430
assertEquals(1, parsedInput.getTools().size());
@@ -959,6 +961,41 @@ public void testCombinedLLMAndMemoryPartialUpdates() {
959961
assertEquals(Integer.valueOf(10), updatedAgent.getMemory().getWindowSize()); // Updated
960962
}
961963

964+
@Test
965+
public void testAgentTypeValidation() {
966+
MLAgent originalAgent = MLAgent.builder().type(MLAgentType.FLOW.name()).name("Test Agent").build();
967+
968+
// Same type should be allowed
969+
MLAgentUpdateInput sameTypeInput = MLAgentUpdateInput
970+
.builder()
971+
.agentId("test-agent-id")
972+
.type(MLAgentType.FLOW.name())
973+
.name("Updated Name")
974+
.build();
975+
976+
MLAgent updatedAgent = sameTypeInput.toMLAgent(originalAgent);
977+
assertEquals(MLAgentType.FLOW.name(), updatedAgent.getType());
978+
assertEquals("Updated Name", updatedAgent.getName());
979+
980+
// Different type should throw error
981+
MLAgentUpdateInput differentTypeInput = MLAgentUpdateInput
982+
.builder()
983+
.agentId("test-agent-id")
984+
.type(MLAgentType.CONVERSATIONAL.name())
985+
.name("Updated Name")
986+
.build();
987+
988+
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> { differentTypeInput.toMLAgent(originalAgent); });
989+
assertEquals("Agent type cannot be updated", e.getMessage());
990+
991+
// No type provided should work (original type)
992+
MLAgentUpdateInput noTypeInput = MLAgentUpdateInput.builder().agentId("test-agent-id").name("Updated Name").build();
993+
994+
MLAgent originalAgentType = noTypeInput.toMLAgent(originalAgent);
995+
assertEquals(MLAgentType.FLOW.name(), originalAgentType.getType());
996+
assertEquals("Updated Name", originalAgentType.getName());
997+
}
998+
962999
@Test
9631000
public void testStreamInputOutputWithVersion() throws IOException {
9641001
MLAgentUpdateInput input = MLAgentUpdateInput

‎gradle/wrapper/gradle-wrapper.jar‎

181 Bytes
Binary file not shown.

‎gradle/wrapper/gradle-wrapper.properties‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
distributionBase=GRADLE_USER_HOME
22
distributionPath=wrapper/dists
3-
distributionSha256Sum=ed1a8d686605fd7c23bdf62c7fc7add1c5b23b2bbc3721e661934ef4a4911d7c
4-
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.3-all.zip
3+
distributionSha256Sum=16f2b95838c1ddcf7242b1c39e7bbbb43c842f1f1a1a0dc4959b6d4d68abcac3
4+
distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-all.zip
55
networkTimeout=10000
66
validateDistributionUrl=true
77
zipStoreBase=GRADLE_USER_HOME

‎gradlew‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ done
8686
# shellcheck disable=SC2034
8787
APP_BASE_NAME=${0##*/}
8888
# Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036)
89-
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s
90-
' "$PWD" ) || exit
89+
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit
9190

9291
# Use the maximum available, or set MAX_FD != -1 to use that value.
9392
MAX_FD=maximum
@@ -115,7 +114,7 @@ case "$( uname )" in #(
115114
NONSTOP* ) nonstop=true ;;
116115
esac
117116

118-
CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar
117+
CLASSPATH="\\\"\\\""
119118

120119

121120
# Determine the Java command to use to start the JVM.
@@ -206,15 +205,15 @@ fi
206205
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
207206

208207
# Collect all arguments for the java command:
209-
# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
208+
# * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
210209
# and any embedded shellness will be escaped.
211210
# * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
212211
# treated as '${Hostname}' itself on the command line.
213212

214213
set -- \
215214
"-Dorg.gradle.appname=$APP_BASE_NAME" \
216215
-classpath "$CLASSPATH" \
217-
org.gradle.wrapper.GradleWrapperMain \
216+
-jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \
218217
"$@"
219218

220219
# Stop when "xargs" is not available.

0 commit comments

Comments
 (0)