Skip to content

Commit

Permalink
fix breaking changes in config index fields (#2882)
Browse files Browse the repository at this point in the history 
* fix breaking changes in config index fields

Signed-off-by: Bhavana Ramaram <[email protected]>
(cherry picked from commit cc402b3)
  • Loading branch information
@rbhavna @github-actions
rbhavna authored and github-actions[bot] committed Sep 4, 2024
1 parent 54d7daf commit 04c7335
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 13 deletions.
1 change: 1 addition & 0 deletions common/src/main/java/org/opensearch/ml/common/CommonValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ public class CommonValue {
public static final Version VERSION_2_12_0 = Version.fromString("2.12.0");
public static final Version VERSION_2_13_0 = Version.fromString("2.13.0");
public static final Version VERSION_2_14_0 = Version.fromString("2.14.0");
public static final Version VERSION_2_15_0 = Version.fromString("2.15.0");
public static final Version VERSION_2_16_0 = Version.fromString("2.16.0");
public static final Version VERSION_2_17_0 = Version.fromString("2.17.0");
}
74 changes: 64 additions & 10 deletions common/src/main/java/org/opensearch/ml/common/MLConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.io.IOException;
import java.time.Instant;

import org.opensearch.Version;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable;
Expand All @@ -28,43 +29,74 @@ public class MLConfig implements ToXContentObject, Writeable {

public static final String TYPE_FIELD = "type";

public static final String CONFIG_TYPE_FIELD = "config_type";

public static final String CONFIGURATION_FIELD = "configuration";

public static final String ML_CONFIGURATION_FIELD = "ml_configuration";

public static final String CREATE_TIME_FIELD = "create_time";
public static final String LAST_UPDATE_TIME_FIELD = "last_update_time";

// Adding below three new fields since the original fields, type, configuration, and last_update_time
// are not created with correct data types in config index due to missing schema version bump.
// Starting 2.15, it is suggested that below fields be used for creating new documents in config index

public static final String CONFIG_TYPE_FIELD = "config_type";

public static final String ML_CONFIGURATION_FIELD = "ml_configuration";

public static final String LAST_UPDATED_TIME_FIELD = "last_updated_time";

private static final Version MINIMAL_SUPPORTED_VERSION_FOR_NEW_CONFIG_FIELDS = CommonValue.VERSION_2_15_0;

@Setter
private String type;

@Setter
private String configType;

private Configuration configuration;
private Configuration mlConfiguration;
private final Instant createTime;
private Instant lastUpdateTime;
private Instant lastUpdatedTime;

@Builder(toBuilder = true)
public MLConfig(String type, Configuration configuration, Instant createTime, Instant lastUpdateTime) {
public MLConfig(
String type,
String configType,
Configuration configuration,
Configuration mlConfiguration,
Instant createTime,
Instant lastUpdateTime,
Instant lastUpdatedTime
) {
this.type = type;
this.configType = configType;
this.configuration = configuration;
this.mlConfiguration = mlConfiguration;
this.createTime = createTime;
this.lastUpdateTime = lastUpdateTime;
this.lastUpdatedTime = lastUpdatedTime;
}

public MLConfig(StreamInput input) throws IOException {
Version streamInputVersion = input.getVersion();
this.type = input.readOptionalString();
if (input.readBoolean()) {
configuration = new Configuration(input);
}
createTime = input.readOptionalInstant();
lastUpdateTime = input.readOptionalInstant();
if (streamInputVersion.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_NEW_CONFIG_FIELDS)) {
this.configType = input.readOptionalString();
if (input.readBoolean()) {
mlConfiguration = new Configuration(input);
}
lastUpdatedTime = input.readOptionalInstant();
}
}

@Override
public void writeTo(StreamOutput out) throws IOException {
Version streamOutputVersion = out.getVersion();
out.writeOptionalString(type);
if (configuration != null) {
out.writeBoolean(true);
Expand All @@ -74,23 +106,42 @@ public void writeTo(StreamOutput out) throws IOException {
}
out.writeOptionalInstant(createTime);
out.writeOptionalInstant(lastUpdateTime);
if (streamOutputVersion.onOrAfter(MINIMAL_SUPPORTED_VERSION_FOR_NEW_CONFIG_FIELDS)) {
out.writeOptionalString(configType);
if (mlConfiguration != null) {
out.writeBoolean(true);
mlConfiguration.writeTo(out);
} else {
out.writeBoolean(false);
}
out.writeOptionalInstant(lastUpdatedTime);
}
}

@Override
public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException {
XContentBuilder builder = xContentBuilder.startObject();
if (type != null) {
builder.field(CONFIG_TYPE_FIELD, type);
builder.field(TYPE_FIELD, type);
}
if (configType != null) {
builder.field(CONFIG_TYPE_FIELD, configType);
}
if (configuration != null) {
builder.field(ML_CONFIGURATION_FIELD, configuration);
builder.field(CONFIGURATION_FIELD, configuration);
}
if (mlConfiguration != null) {
builder.field(ML_CONFIGURATION_FIELD, mlConfiguration);
}
if (createTime != null) {
builder.field(CREATE_TIME_FIELD, createTime.toEpochMilli());
}
if (lastUpdateTime != null) {
builder.field(LAST_UPDATE_TIME_FIELD, lastUpdateTime.toEpochMilli());
}
if (lastUpdatedTime != null) {
builder.field(LAST_UPDATED_TIME_FIELD, lastUpdatedTime.toEpochMilli());
}
return builder.endObject();
}

Expand Down Expand Up @@ -142,10 +193,13 @@ public static MLConfig parse(XContentParser parser) throws IOException {
}
return MLConfig
.builder()
.type(configType == null ? type : configType)
.configuration(mlConfiguration == null ? configuration : mlConfiguration)
.type(type)
.configType(configType)
.configuration(configuration)
.mlConfiguration(mlConfiguration)
.createTime(createTime)
.lastUpdateTime(lastUpdatedTime == null ? lastUpdateTime : lastUpdatedTime)
.lastUpdateTime(lastUpdateTime)
.lastUpdatedTime(lastUpdatedTime)
.build();
}
}
4 changes: 2 additions & 2 deletions common/src/test/java/org/opensearch/ml/common/transport/config/MLConfigGetResponseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void MLConfigGetResponse_Builder() throws IOException {
@Test
public void writeTo() throws IOException {
// create ml agent using mlConfig and mlConfigGetResponse
mlConfig = new MLConfig("olly_agent", new Configuration("agent_id"), Instant.EPOCH, Instant.EPOCH);
mlConfig = new MLConfig("olly_agent", null, new Configuration("agent_id"), null, Instant.EPOCH, Instant.EPOCH, Instant.EPOCH);
MLConfigGetResponse mlConfigGetResponse = MLConfigGetResponse.builder().mlConfig(mlConfig).build();
// use write out for both agents
BytesStreamOutput output = new BytesStreamOutput();
Expand All @@ -76,7 +76,7 @@ public void writeTo() throws IOException {

@Test
public void toXContent() throws IOException {
mlConfig = new MLConfig(null, null, null, null);
mlConfig = new MLConfig(null, null, null, null, null, null, null);
MLConfigGetResponse mlConfigGetResponse = MLConfigGetResponse.builder().mlConfig(mlConfig).build();
XContentBuilder builder = XContentFactory.jsonBuilder();
ToXContent.Params params = EMPTY_PARAMS;
Expand Down
25 changes: 24 additions & 1 deletion plugin/src/test/java/org/opensearch/ml/action/config/GetConfigTransportActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,32 @@ public void testDoExecute_Success() throws IOException {
verify(actionListener).onResponse(any(MLConfigGetResponse.class));
}

@Test
public void testDoExecute_Success_ForNewFields() throws IOException {
String configID = "config_id";
MLConfig mlConfig = new MLConfig(null, "olly_agent", null, new Configuration("agent_id"), Instant.EPOCH, null, Instant.EPOCH);

XContentBuilder content = mlConfig.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS);
BytesReference bytesReference = BytesReference.bytes(content);
GetResult getResult = new GetResult("indexName", configID, 111l, 111l, 111l, true, bytesReference, null, null);
GetResponse getResponse = new GetResponse(getResult);
ActionListener<MLConfigGetResponse> actionListener = mock(ActionListener.class);
MLConfigGetRequest request = new MLConfigGetRequest(configID);
Task task = mock(Task.class);

doAnswer(invocation -> {
ActionListener<GetResponse> listener = invocation.getArgument(1);
listener.onResponse(getResponse);
return null;
}).when(client).get(any(), any());

getConfigTransportAction.doExecute(task, request, actionListener);
verify(actionListener).onResponse(any(MLConfigGetResponse.class));
}

public GetResponse prepareMLConfig(String configID) throws IOException {

MLConfig mlConfig = new MLConfig("olly_agent", new Configuration("agent_id"), Instant.EPOCH, Instant.EPOCH);
MLConfig mlConfig = new MLConfig("olly_agent", null, new Configuration("agent_id"), null, Instant.EPOCH, Instant.EPOCH, null);

XContentBuilder content = mlConfig.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS);
BytesReference bytesReference = BytesReference.bytes(content);
Expand Down

0 comments on commit 04c7335

Please sign in to comment.