Skip to content

Commit

Permalink
Prevents Locale Code build failures #2967 (#2980)
Browse files Browse the repository at this point in the history
* Prevents future UT build issues addresses #2967

Depending on the GitHub CI workflow executes it will invoke ./gradlew build which will run tests on random parameters everytime, there are local-code's that will break a build one of them being az-Cyrl. The solution here was preventing (explicitly writing the local to be set to en-US) Github actions building on these random parameters that will interupt a build and lose time. Manual testing was done to prove that using this flag breaks a build, you can check the issue #2967 or you can run ./gradlew build -Dtests.local=az-Cyrl to see it breaks

Signed-off-by: Brian Flores <[email protected]>

* Updates current codebase with Local.Root to string operations

The previous commit made a hard change on the build while ignoring the root problem, which was making sure that our codebase currently supports string operations regardless of the locale code. In this new commit String operations like toUpperCase have a extra argument of Locale.Root making the codebase agnostic to the rules of other langugages such as Spanish or Turkish. Manual testing was done like raised in the GitHub issue #2967 also ./gradlew build -Dtests.Local=az-Cyrl passes

Signed-off-by: Brian Flores <[email protected]>

---------

Signed-off-by: Brian Flores <[email protected]>
  • Loading branch information
brianf-aws authored Sep 26, 2024
1 parent 107b916 commit cd83590
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public static boolean isValidActionInModelPrediction(ActionType actionType) {

public static boolean isValidAction(String action) {
try {
ActionType.valueOf(action.toUpperCase());
ActionType.valueOf(action.toUpperCase(Locale.ROOT));
return true;
} catch (IllegalArgumentException e) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;

import java.io.IOException;
import java.util.Locale;

import org.opensearch.core.ParseField;
import org.opensearch.core.common.io.stream.StreamInput;
Expand Down Expand Up @@ -97,7 +98,7 @@ public static MLAlgoParams parse(XContentParser parser) throws IOException {
parallel = parser.booleanValue();
break;
case DISTANCE_TYPE_FIELD:
distanceType = DistanceType.from(parser.text().toUpperCase());
distanceType = DistanceType.from(parser.text().toUpperCase(Locale.ROOT));
break;
default:
parser.skipChildren();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;

import org.opensearch.ResourceNotFoundException;
Expand Down Expand Up @@ -281,7 +282,7 @@ private ActionListener<Object> createAgentActionListener(

@VisibleForTesting
protected MLAgentRunner getAgentRunner(MLAgent mlAgent) {
final MLAgentType agentType = MLAgentType.from(mlAgent.getType().toUpperCase());
final MLAgentType agentType = MLAgentType.from(mlAgent.getType().toUpperCase(Locale.ROOT));
switch (agentType) {
case FLOW:
return new MLFlowAgentRunner(client, settings, clusterService, xContentRegistry, toolFactories, memoryFactoryMap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.time.Instant;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -101,7 +102,7 @@ protected void doExecute(Task task, ActionRequest request, ActionListener<MLBatc
mlTaskManager.add(mlTask);
listener.onResponse(new MLBatchIngestionResponse(taskId, MLTaskType.BATCH_INGEST, MLTaskState.CREATED.name()));
String ingestType = (String) mlBatchIngestionInput.getDataSources().get(TYPE);
Ingestable ingestable = MLEngineClassLoader.initInstance(ingestType.toLowerCase(), client, Client.class);
Ingestable ingestable = MLEngineClassLoader.initInstance(ingestType.toLowerCase(Locale.ROOT), client, Client.class);
threadPool.executor(INGEST_THREAD_POOL).execute(() -> {
executeWithErrorHandling(() -> {
double successRate = ingestable.ingest(mlBatchIngestionInput);
Expand Down Expand Up @@ -193,7 +194,7 @@ private void validateBatchIngestInput(MLBatchIngestionInput mlBatchIngestionInpu
if (dataSources.get(TYPE) == null || dataSources.get(SOURCE) == null) {
throw new IllegalArgumentException("The batch ingest input data source is missing data type or source");
}
if (((String) dataSources.get(TYPE)).toLowerCase() == "s3") {
if (((String) dataSources.get(TYPE)).equalsIgnoreCase("s3")) {
List<String> s3Uris = (List<String>) dataSources.get(SOURCE);
if (s3Uris == null || s3Uris.isEmpty()) {
throw new IllegalArgumentException("The batch ingest input s3Uris is empty");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ MLPredictionTaskRequest getRequest(String modelId, String algorithm, RestRequest
ActionType actionType = ActionType.from(getActionTypeFromRestRequest(request));
if (FunctionName.REMOTE.name().equals(algorithm) && !mlFeatureEnabledSetting.isRemoteInferenceEnabled()) {
throw new IllegalStateException(REMOTE_INFERENCE_DISABLED_ERR_MSG);
} else if (FunctionName.isDLModel(FunctionName.from(algorithm.toUpperCase())) && !mlFeatureEnabledSetting.isLocalModelEnabled()) {
} else if (FunctionName.isDLModel(FunctionName.from(algorithm.toUpperCase(Locale.ROOT)))
&& !mlFeatureEnabledSetting.isLocalModelEnabled()) {
throw new IllegalStateException(LOCAL_MODEL_DISABLED_ERR_MSG);
} else if (ActionType.BATCH_PREDICT == actionType && !mlFeatureEnabledSetting.isOfflineBatchInferenceEnabled()) {
throw new IllegalStateException(BATCH_INFERENCE_DISABLED_ERR_MSG);
Expand Down

0 comments on commit cd83590

Please sign in to comment.