Skip to content

Commit

Permalink
Adding String.format for exception error messages
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Gaievski <[email protected]>
  • Loading branch information
martin-gaievski committed Jul 14, 2022
1 parent ead2b6d commit efd487e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,13 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
}
int value = XContentMapValues.nodeIntegerValue(o);
if (value > MAX_DIMENSION) {
throw new IllegalArgumentException("Dimension value cannot be greater than " + MAX_DIMENSION + " for vector: " + name);
throw new IllegalArgumentException(
String.format("Dimension value cannot be greater than %s for vector: %s", MAX_DIMENSION, name)
);
}

if (value <= 0) {
throw new IllegalArgumentException("Dimension value must be greater than 0 " + "for vector: " + name);
throw new IllegalArgumentException(String.format("Dimension value must be greater than 0 for vector: %s", name));
}
return value;
}, m -> toType(m).dimension);
Expand Down Expand Up @@ -273,12 +275,12 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
// is done before any mappers are built. Therefore, validation should be done during parsing
// so that it can fail early.
if (builder.knnMethodContext.get() != null && builder.modelId.get() != null) {
throw new IllegalArgumentException("Method and model can not be both specified in the mapping: " + name);
throw new IllegalArgumentException(String.format("Method and model can not be both specified in the mapping: %s", name));
}

// Dimension should not be null unless modelId is used
if (builder.dimension.getValue() == -1 && builder.modelId.get() == null) {
throw new IllegalArgumentException("Dimension value missing for vector: " + name);
throw new IllegalArgumentException(String.format("Dimension value missing for vector: %s", name));
}

return builder;
Expand Down Expand Up @@ -325,7 +327,7 @@ public Query existsQuery(QueryShardContext context) {
public Query termQuery(Object value, QueryShardContext context) {
throw new QueryShardException(
context,
"KNN vector do not support exact searching, use KNN queries " + "instead: [" + name() + "]"
String.format("KNN vector do not support exact searching, use KNN queries instead: [%s]", name())
);
}

Expand Down Expand Up @@ -401,14 +403,14 @@ protected void parseCreateField(ParseContext context, int dimension) throws IOEx
void validateIfCircuitBreakerIsNotTriggered() {
if (KNNSettings.isCircuitBreakerTriggered()) {
throw new IllegalStateException(
"Indexing knn vector fields is rejected as circuit breaker triggered." + " Check _opendistro/_knn/stats for detailed state"
"Indexing knn vector fields is rejected as circuit breaker triggered. Check _opendistro/_knn/stats for detailed state"
);
}
}

void validateIfKNNPluginEnabled() {
if (!KNNSettings.isKNNPluginEnabled()) {
throw new IllegalStateException("KNN plugin is disabled. To enable " + "update knn.plugin.enabled setting to true");
throw new IllegalStateException("KNN plugin is disabled. To enable update knn.plugin.enabled setting to true");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@
import static org.opensearch.knn.common.KNNConstants.SPACE_TYPE;

/**
* Field mapper for original implementation
* Field mapper for original implementation. It defaults to using nmslib as the engine and retrieves parameters from index settings.
*
* Example of this mapper output:
*
* {
* "type": "knn_vector",
* "dimension": 128
* }
*/
@Log4j2
public class LegacyFieldMapper extends KNNVectorFieldMapper {
Expand Down Expand Up @@ -70,11 +77,11 @@ static String getSpaceType(Settings indexSettings) {
String spaceType = indexSettings.get(KNNSettings.INDEX_KNN_SPACE_TYPE.getKey());
if (spaceType == null) {
log.info(
"[KNN] The setting \""
+ METHOD_PARAMETER_SPACE_TYPE
+ "\" was not set for the index. "
+ "Likely caused by recent version upgrade. Setting the setting to the default value="
+ KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE
String.format(
"[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s",
METHOD_PARAMETER_SPACE_TYPE,
KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE
)
);
return KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE;
}
Expand All @@ -85,11 +92,11 @@ static String getM(Settings indexSettings) {
String m = indexSettings.get(KNNSettings.INDEX_KNN_ALGO_PARAM_M_SETTING.getKey());
if (m == null) {
log.info(
"[KNN] The setting \""
+ HNSW_ALGO_M
+ "\" was not set for the index. "
+ "Likely caused by recent version upgrade. Setting the setting to the default value="
+ KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M
String.format(
"[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s",
HNSW_ALGO_M,
KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M
)
);
return String.valueOf(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_M);
}
Expand All @@ -100,11 +107,11 @@ static String getEfConstruction(Settings indexSettings) {
String efConstruction = indexSettings.get(KNNSettings.INDEX_KNN_ALGO_PARAM_EF_CONSTRUCTION_SETTING.getKey());
if (efConstruction == null) {
log.info(
"[KNN] The setting \""
+ HNSW_ALGO_EF_CONSTRUCTION
+ "\" was not set for"
+ " the index. Likely caused by recent version upgrade. Setting the setting to the default value="
+ KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION
String.format(
"[KNN] The setting \"%s\" was not set for the index. Likely caused by recent version upgrade. Setting the setting to the default value=%s",
HNSW_ALGO_EF_CONSTRUCTION,
KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION
)
);
return String.valueOf(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class MethodFieldMapper extends KNNVectorFieldMapper {
Strings.toString(XContentFactory.jsonBuilder().map(knnEngine.getMethodAsMap(knnMethodContext)))
);
} catch (IOException ioe) {
throw new RuntimeException("Unable to create KNNVectorFieldMapper: " + ioe);
throw new RuntimeException(String.format("Unable to create KNNVectorFieldMapper: %s", ioe));
}

this.fieldType.freeze();
Expand Down

0 comments on commit efd487e

Please sign in to comment.