Skip to content

Commit

Permalink
Handle Not Found deprovision exceptions as successful deletions (#805)
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Widdis <[email protected]>
  • Loading branch information
dbwiddis authored Jul 25, 2024
1 parent 761edd9 commit 034dff7
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
- Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors ([#750](https://github.com/opensearch-project/flow-framework/pull/750))

### Bug Fixes
- Handle Not Found exceptions as successful deletions for agents and models ([#805](https://github.com/opensearch-project/flow-framework/pull/805))

### Infrastructure
### Documentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Compatible with OpenSearch 2.16.0
- Add allow_delete parameter to Deprovision API ([#763](https://github.com/opensearch-project/flow-framework/pull/763))
- Improve Template and WorkflowState builders ([#778](https://github.com/opensearch-project/flow-framework/pull/778))

### Bug Fixes
- Handle Not Found deprovision exceptions as successful deletions ([#805](https://github.com/opensearch-project/flow-framework/pull/805))

### Infrastructure
- Update dependency com.fasterxml.jackson.core:jackson-core to v2.17.2 ([#760](https://github.com/opensearch-project/flow-framework/pull/760))
- Update dependency gradle to v8.8 ([#725](https://github.com/opensearch-project/flow-framework/pull/725))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.ExceptionsHelper;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.delete.DeleteResponse;
import org.opensearch.action.support.PlainActionFuture;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.flowframework.exception.WorkflowStepException;
import org.opensearch.flowframework.util.ParseUtils;
Expand Down Expand Up @@ -85,9 +87,21 @@ public void onResponse(DeleteResponse deleteResponse) {
@Override
public void onFailure(Exception ex) {
Exception e = getSafeException(ex);
String errorMessage = (e == null ? "Failed to delete agent " + agentId : e.getMessage());
logger.error(errorMessage, e);
deleteAgentFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
// NOT_FOUND exception should be treated as successful deletion
// https://github.com/opensearch-project/ml-commons/issues/2751
if (e instanceof OpenSearchStatusException && ((OpenSearchStatusException) e).status().equals(RestStatus.NOT_FOUND)) {
deleteAgentFuture.onResponse(
new WorkflowData(
Map.ofEntries(Map.entry(AGENT_ID, agentId)),
currentNodeInputs.getWorkflowId(),
currentNodeInputs.getNodeId()
)
);
} else {
String errorMessage = (e == null ? "Failed to delete agent " + agentId : e.getMessage());
logger.error(errorMessage, e);
deleteAgentFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
}
}
});
} catch (FlowFrameworkException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.ExceptionsHelper;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.delete.DeleteResponse;
import org.opensearch.action.support.PlainActionFuture;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.flowframework.exception.WorkflowStepException;
import org.opensearch.flowframework.util.ParseUtils;
Expand Down Expand Up @@ -86,9 +88,21 @@ public void onResponse(DeleteResponse deleteResponse) {
@Override
public void onFailure(Exception ex) {
Exception e = getSafeException(ex);
String errorMessage = (e == null ? "Failed to delete model " + modelId : e.getMessage());
logger.error(errorMessage, e);
deleteModelFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
// NOT_FOUND exception should be treated as successful deletion
// https://github.com/opensearch-project/ml-commons/issues/2751
if (e instanceof OpenSearchStatusException && ((OpenSearchStatusException) e).status().equals(RestStatus.NOT_FOUND)) {
deleteModelFuture.onResponse(
new WorkflowData(
Map.ofEntries(Map.entry(MODEL_ID, modelId)),
currentNodeInputs.getWorkflowId(),
currentNodeInputs.getNodeId()
)
);
} else {
String errorMessage = (e == null ? "Failed to delete model " + modelId : e.getMessage());
logger.error(errorMessage, e);
deleteModelFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
}
}
});
} catch (FlowFrameworkException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
package org.opensearch.flowframework.workflow;

import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.delete.DeleteResponse;
import org.opensearch.action.support.PlainActionFuture;
import org.opensearch.core.action.ActionListener;
Expand Down Expand Up @@ -73,6 +74,30 @@ public void testDeleteAgent() throws IOException, ExecutionException, Interrupte
assertEquals(agentId, future.get().getContent().get(AGENT_ID));
}

public void testDeleteAgentNotFound() throws IOException, ExecutionException, InterruptedException {

String agentId = randomAlphaOfLength(5);
DeleteAgentStep deleteAgentStep = new DeleteAgentStep(machineLearningNodeClient);

doAnswer(invocation -> {
ActionListener<DeleteResponse> actionListener = invocation.getArgument(1);
actionListener.onFailure(new OpenSearchStatusException("No agent found with that id", RestStatus.NOT_FOUND));
return null;
}).when(machineLearningNodeClient).deleteAgent(any(String.class), any());

PlainActionFuture<WorkflowData> future = deleteAgentStep.execute(
inputData.getNodeId(),
inputData,
Map.of("step_1", new WorkflowData(Map.of(AGENT_ID, agentId), "workflowId", "nodeId")),
Map.of("step_1", AGENT_ID),
Collections.emptyMap()
);
verify(machineLearningNodeClient).deleteAgent(any(String.class), any());

assertTrue(future.isDone());
assertEquals(agentId, future.get().getContent().get(AGENT_ID));
}

public void testNoAgentIdInOutput() throws IOException {
DeleteAgentStep deleteAgentStep = new DeleteAgentStep(machineLearningNodeClient);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
package org.opensearch.flowframework.workflow;

import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.delete.DeleteResponse;
import org.opensearch.action.support.PlainActionFuture;
import org.opensearch.core.action.ActionListener;
Expand Down Expand Up @@ -73,6 +74,30 @@ public void testDeleteModel() throws IOException, ExecutionException, Interrupte
assertEquals(modelId, future.get().getContent().get(MODEL_ID));
}

public void testDeleteModelNotFound() throws IOException, ExecutionException, InterruptedException {

String modelId = randomAlphaOfLength(5);
DeleteModelStep deleteModelStep = new DeleteModelStep(machineLearningNodeClient);

doAnswer(invocation -> {
ActionListener<DeleteResponse> actionListener = invocation.getArgument(1);
actionListener.onFailure(new OpenSearchStatusException("No model found with that id", RestStatus.NOT_FOUND));
return null;
}).when(machineLearningNodeClient).deleteModel(any(String.class), any());

PlainActionFuture<WorkflowData> future = deleteModelStep.execute(
inputData.getNodeId(),
inputData,
Map.of("step_1", new WorkflowData(Map.of(MODEL_ID, modelId), "workflowId", "nodeId")),
Map.of("step_1", MODEL_ID),
Collections.emptyMap()
);
verify(machineLearningNodeClient).deleteModel(any(String.class), any());

assertTrue(future.isDone());
assertEquals(modelId, future.get().getContent().get(MODEL_ID));
}

public void testNoModelIdInOutput() throws IOException {
DeleteModelStep deleteModelStep = new DeleteModelStep(machineLearningNodeClient);

Expand Down

0 comments on commit 034dff7

Please sign in to comment.