Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/142738.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 142738
summary: Added service settings update logic for Alibaba Cloud Search provider in the Inference Plugin
area: Inference
type: enhancement
issues:
- 122356
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public AlibabaCloudSearchModel buildModelFromConfigAndSecrets(ModelConfiguration
config.getInferenceEntityId(),
config.getTaskType(),
config.getService(),
ConfigurationParseContext.PERSISTENT
ConfigurationParseContext.REQUEST
).createFromModelConfigurationsAndSecrets(config, secrets);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,37 +38,37 @@ public class AlibabaCloudSearchServiceSettings extends FilteredXContentObject
public static final String HOST = "host";
public static final String WORKSPACE_NAME = "workspace";
public static final String HTTP_SCHEMA_NAME = "http_schema";
private static final Set<String> VALID_SCHEMAS = Set.of("https", "http");

private static final RateLimitSettings DEFAULT_RATE_LIMIT_SETTINGS = new RateLimitSettings(1_000);

public static AlibabaCloudSearchServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
ValidationException validationException = new ValidationException();
var validationException = new ValidationException();

String modelId = extractRequiredString(map, SERVICE_ID, ModelConfigurations.SERVICE_SETTINGS, validationException);
String host = extractRequiredString(map, HOST, ModelConfigurations.SERVICE_SETTINGS, validationException);
var serviceId = extractRequiredString(map, SERVICE_ID, ModelConfigurations.SERVICE_SETTINGS, validationException);
var host = extractRequiredString(map, HOST, ModelConfigurations.SERVICE_SETTINGS, validationException);
var workspaceName = extractRequiredString(map, WORKSPACE_NAME, ModelConfigurations.SERVICE_SETTINGS, validationException);
var httpSchema = extractOptionalString(map, HTTP_SCHEMA_NAME, ModelConfigurations.SERVICE_SETTINGS, validationException);

if (httpSchema != null) {
var validSchemas = Set.of("https", "http");
if (validSchemas.contains(httpSchema) == false) {
validationException.addValidationError("Invalid value for [http_schema]. Must be one of [https, http]");
}
}
validateHttpSchema(httpSchema, validationException);

RateLimitSettings rateLimitSettings = RateLimitSettings.of(
var rateLimitSettings = RateLimitSettings.of(
map,
DEFAULT_RATE_LIMIT_SETTINGS,
validationException,
AlibabaCloudSearchService.NAME,
context
);

if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
}
validationException.throwIfValidationErrorsExist();

return new AlibabaCloudSearchServiceSettings(modelId, host, workspaceName, httpSchema, rateLimitSettings);
return new AlibabaCloudSearchServiceSettings(serviceId, host, workspaceName, httpSchema, rateLimitSettings);
}

static void validateHttpSchema(String httpSchema, ValidationException validationException) {
if (httpSchema != null && VALID_SCHEMAS.contains(httpSchema) == false) {
validationException.addValidationError("Invalid value for [http_schema]. Must be one of [https, http]");
}
}

private final String serviceId;
Expand All @@ -92,18 +92,50 @@ public AlibabaCloudSearchServiceSettings(
}

public AlibabaCloudSearchServiceSettings(StreamInput in) throws IOException {
serviceId = in.readString();
host = in.readString();
workspaceName = in.readString();
httpSchema = in.readOptionalString();
rateLimitSettings = new RateLimitSettings(in);
this.serviceId = in.readString();
this.host = in.readString();
this.workspaceName = in.readString();
this.httpSchema = in.readOptionalString();
this.rateLimitSettings = new RateLimitSettings(in);
}

@Override
public String modelId() {
return serviceId;
}

@Override
public AlibabaCloudSearchServiceSettings updateServiceSettings(Map<String, Object> serviceSettings) {
var validationException = new ValidationException();

var extractedHttpSchema = extractOptionalString(
serviceSettings,
HTTP_SCHEMA_NAME,
ModelConfigurations.SERVICE_SETTINGS,
validationException
);

validateHttpSchema(extractedHttpSchema, validationException);

var extractedRateLimitSettings = RateLimitSettings.of(
serviceSettings,
this.rateLimitSettings,
validationException,
AlibabaCloudSearchService.NAME,
ConfigurationParseContext.REQUEST
);

validationException.throwIfValidationErrorsExist();

return new AlibabaCloudSearchServiceSettings(
this.serviceId,
this.host,
this.workspaceName,
Comment thread
Jan-Kazlouski-elastic marked this conversation as resolved.
extractedHttpSchema != null ? extractedHttpSchema : this.httpSchema,
extractedRateLimitSettings
);
}

public String getHost() {
return host;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package org.elasticsearch.xpack.inference.services.alibabacloudsearch.completion;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.inference.ServiceSettings;
Expand All @@ -25,13 +24,7 @@ public class AlibabaCloudSearchCompletionServiceSettings implements ServiceSetti
public static final String NAME = "alibabacloud_search_completion_service_settings";

public static AlibabaCloudSearchCompletionServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
ValidationException validationException = new ValidationException();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValidationException was not passed anywhere and couldn't be filled out with any validation errors. Actual validation was performed only within the AlibabaCloudSearchServiceSettings.fromMap method

var commonServiceSettings = AlibabaCloudSearchServiceSettings.fromMap(map, context);
if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
}

return new AlibabaCloudSearchCompletionServiceSettings(commonServiceSettings);
return new AlibabaCloudSearchCompletionServiceSettings(AlibabaCloudSearchServiceSettings.fromMap(map, context));
}

private final AlibabaCloudSearchServiceSettings commonSettings;
Expand All @@ -41,7 +34,7 @@ public AlibabaCloudSearchCompletionServiceSettings(AlibabaCloudSearchServiceSett
}

public AlibabaCloudSearchCompletionServiceSettings(StreamInput in) throws IOException {
commonSettings = new AlibabaCloudSearchServiceSettings(in);
this.commonSettings = new AlibabaCloudSearchServiceSettings(in);
}

public AlibabaCloudSearchServiceSettings getCommonSettings() {
Expand All @@ -53,6 +46,11 @@ public String modelId() {
return commonSettings.modelId();
}

@Override
public AlibabaCloudSearchCompletionServiceSettings updateServiceSettings(Map<String, Object> serviceSettings) {
return new AlibabaCloudSearchCompletionServiceSettings(commonSettings.updateServiceSettings(serviceSettings));
}

@Override
public String getWriteableName() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,28 @@
import static org.elasticsearch.xpack.inference.services.ServiceFields.DIMENSIONS;
import static org.elasticsearch.xpack.inference.services.ServiceFields.MAX_INPUT_TOKENS;
import static org.elasticsearch.xpack.inference.services.ServiceFields.SIMILARITY;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.extractOptionalPositiveInteger;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.extractSimilarity;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.removeAsType;

public class AlibabaCloudSearchEmbeddingsServiceSettings implements ServiceSettings {
public static final String NAME = "alibabacloud_search_embeddings_service_settings";

public static AlibabaCloudSearchEmbeddingsServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
ValidationException validationException = new ValidationException();
var validationException = new ValidationException();
var commonServiceSettings = AlibabaCloudSearchServiceSettings.fromMap(map, context);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you may see, the validation is happening in 2 steps here. If AlibabaCloudSearchServiceSettings.fromMap introduces validation fails - it will fail early without checking embedding specific fields and without filling out the potential errors in these fields.
I don't see this approach as optimal. We could alter the signature of the AlibabaCloudSearchServiceSettings.fromMap to take validation exception as param and fill it out with validation errors for the common fields while keeping the responsibility of throwing the exception on task specific classes. I implemented that locally. If it is decided that we would want to go with this approach I can push it here.

CC @DonalEvans

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of passing the ValidationException into AlibabaCloudSearchServiceSettings.fromMap(). The purpose of the way we use ValidationException is to allow us to capture ALL of the validation problems with a given input, so having two places we can throw while parsing a single object feels wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


SimilarityMeasure similarity = extractSimilarity(map, ModelConfigurations.SERVICE_SETTINGS, validationException);
Integer dims = removeAsType(map, DIMENSIONS, Integer.class);
Integer maxInputTokens = removeAsType(map, MAX_INPUT_TOKENS, Integer.class);
var similarity = extractSimilarity(map, ModelConfigurations.SERVICE_SETTINGS, validationException);
var dimensions = extractOptionalPositiveInteger(map, DIMENSIONS, ModelConfigurations.SERVICE_SETTINGS, validationException);
var maxInputTokens = extractOptionalPositiveInteger(
map,
MAX_INPUT_TOKENS,
ModelConfigurations.SERVICE_SETTINGS,
validationException
);

if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
}
validationException.throwIfValidationErrorsExist();

return new AlibabaCloudSearchEmbeddingsServiceSettings(commonServiceSettings, similarity, dims, maxInputTokens);
return new AlibabaCloudSearchEmbeddingsServiceSettings(commonServiceSettings, similarity, dimensions, maxInputTokens);
}

private final AlibabaCloudSearchServiceSettings commonSettings;
Expand All @@ -67,10 +70,10 @@ public AlibabaCloudSearchEmbeddingsServiceSettings(
}

public AlibabaCloudSearchEmbeddingsServiceSettings(StreamInput in) throws IOException {
commonSettings = new AlibabaCloudSearchServiceSettings(in);
similarity = in.readOptionalEnum(SimilarityMeasure.class);
dimensions = in.readOptionalVInt();
maxInputTokens = in.readOptionalVInt();
this.commonSettings = new AlibabaCloudSearchServiceSettings(in);
this.similarity = in.readOptionalEnum(SimilarityMeasure.class);
this.dimensions = in.readOptionalVInt();
this.maxInputTokens = in.readOptionalVInt();
}

public AlibabaCloudSearchServiceSettings getCommonSettings() {
Expand Down Expand Up @@ -105,6 +108,28 @@ public String modelId() {
return commonSettings.modelId();
}

@Override
public AlibabaCloudSearchEmbeddingsServiceSettings updateServiceSettings(Map<String, Object> serviceSettings) {
var validationException = new ValidationException();
var commonServiceSettings = commonSettings.updateServiceSettings(serviceSettings);

var extractedMaxInputTokens = extractOptionalPositiveInteger(
serviceSettings,
MAX_INPUT_TOKENS,
ModelConfigurations.SERVICE_SETTINGS,
validationException
);

validationException.throwIfValidationErrorsExist();

return new AlibabaCloudSearchEmbeddingsServiceSettings(
commonServiceSettings,
this.similarity,
this.dimensions,
extractedMaxInputTokens != null ? extractedMaxInputTokens : this.maxInputTokens
);
}

@Override
public String getWriteableName() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package org.elasticsearch.xpack.inference.services.alibabacloudsearch.rerank;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.inference.ServiceSettings;
Expand All @@ -25,13 +24,7 @@ public class AlibabaCloudSearchRerankServiceSettings implements ServiceSettings
public static final String NAME = "alibabacloud_search_rerank_service_settings";

public static AlibabaCloudSearchRerankServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
ValidationException validationException = new ValidationException();
var commonServiceSettings = AlibabaCloudSearchServiceSettings.fromMap(map, context);
if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
}

return new AlibabaCloudSearchRerankServiceSettings(commonServiceSettings);
return new AlibabaCloudSearchRerankServiceSettings(AlibabaCloudSearchServiceSettings.fromMap(map, context));
}

private final AlibabaCloudSearchServiceSettings commonSettings;
Expand All @@ -53,6 +46,11 @@ public String modelId() {
return commonSettings.modelId();
}

@Override
public AlibabaCloudSearchRerankServiceSettings updateServiceSettings(Map<String, Object> serviceSettings) {
return new AlibabaCloudSearchRerankServiceSettings(commonSettings.updateServiceSettings(serviceSettings));
}

@Override
public String getWriteableName() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package org.elasticsearch.xpack.inference.services.alibabacloudsearch.sparse;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.inference.ServiceSettings;
Expand All @@ -25,13 +24,7 @@ public class AlibabaCloudSearchSparseServiceSettings implements ServiceSettings
public static final String NAME = "alibabacloud_search_sparse_embeddings_service_settings";

public static AlibabaCloudSearchSparseServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
ValidationException validationException = new ValidationException();
var commonServiceSettings = AlibabaCloudSearchServiceSettings.fromMap(map, context);
if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
}

return new AlibabaCloudSearchSparseServiceSettings(commonServiceSettings);
return new AlibabaCloudSearchSparseServiceSettings(AlibabaCloudSearchServiceSettings.fromMap(map, context));
}

private final AlibabaCloudSearchServiceSettings commonSettings;
Expand All @@ -41,7 +34,7 @@ public AlibabaCloudSearchSparseServiceSettings(AlibabaCloudSearchServiceSettings
}

public AlibabaCloudSearchSparseServiceSettings(StreamInput in) throws IOException {
commonSettings = new AlibabaCloudSearchServiceSettings(in);
this.commonSettings = new AlibabaCloudSearchServiceSettings(in);
}

public AlibabaCloudSearchServiceSettings getCommonSettings() {
Expand All @@ -53,6 +46,11 @@ public String modelId() {
return commonSettings.modelId();
}

@Override
public AlibabaCloudSearchSparseServiceSettings updateServiceSettings(Map<String, Object> serviceSettings) {
return new AlibabaCloudSearchSparseServiceSettings(commonSettings.updateServiceSettings(serviceSettings));
}

@Override
public String getWriteableName() {
return NAME;
Expand Down
Loading