Addressing PR comments

Signed-off-by: Joshua Palis <[email protected]>
joshpalis · Aug 2, 2024 · 3753aec · 3753aec
1 parent 411cd47
commit 3753aec
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 25 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
@@ -111,52 +111,32 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             );
         }
         if (!provision && !params.isEmpty()) {
-            // Consume params and content so custom exception is processed
-            params.keySet().stream().forEach(request::param);
-            request.content();
             FlowFrameworkException ffe = new FlowFrameworkException(
                 "Only the parameters " + request.consumedParams() + " are permitted unless the provision parameter is set to true.",
                 RestStatus.BAD_REQUEST
             );
-            return channel -> channel.sendResponse(
-                new BytesRestResponse(ffe.getRestStatus(), ffe.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS))
-            );
+            return processError(ffe, params, request);
         }
         if (provision && updateFields) {
-            // Consume params and content so custom exception is processed
-            params.keySet().stream().forEach(request::param);
-            request.content();
             FlowFrameworkException ffe = new FlowFrameworkException(
                 "You can not use both the " + PROVISION_WORKFLOW + " and " + UPDATE_WORKFLOW_FIELDS + " parameters in the same request.",
                 RestStatus.BAD_REQUEST
             );
-            return channel -> channel.sendResponse(
-                new BytesRestResponse(ffe.getRestStatus(), ffe.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS))
-            );
+            return processError(ffe, params, request);
         }
         if (reprovision && workflowId == null) {
-            // Consume params and content so custom exception is processed
-            params.keySet().stream().forEach(request::param);
-            request.content();
             FlowFrameworkException ffe = new FlowFrameworkException(
                 "You can not use the " + REPROVISION_WORKFLOW + " parameter to create a new template.",
                 RestStatus.BAD_REQUEST
             );
-            return channel -> channel.sendResponse(
-                new BytesRestResponse(ffe.getRestStatus(), ffe.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS))
-            );
+            return processError(ffe, params, request);
         }
         if (reprovision && useCase != null) {
-            // Consume params and content so custom exception is processed
-            params.keySet().stream().forEach(request::param);
-            request.content();
             FlowFrameworkException ffe = new FlowFrameworkException(
                 "You cannot use the " + REPROVISION_WORKFLOW + " and " + USE_CASE + " parameters in the same request.",
                 RestStatus.BAD_REQUEST
             );
-            return channel -> channel.sendResponse(
-                new BytesRestResponse(ffe.getRestStatus(), ffe.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS))
-            );
+            return processError(ffe, params, request);
         }
         try {
             Template template;
@@ -277,4 +257,13 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             );
         }
     }
+
+    private RestChannelConsumer processError(FlowFrameworkException ffe, Map<String, String> params, RestRequest request) {
+        // Consume params and content so custom exception is processed
+        params.keySet().stream().forEach(request::param);
+        request.content();
+        return channel -> channel.sendResponse(
+            new BytesRestResponse(ffe.getRestStatus(), ffe.toXContent(channel.newErrorBuilder(), ToXContent.EMPTY_PARAMS))
+        );
+    }
 }
diff --git a/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
@@ -135,7 +135,8 @@ public PlainActionFuture<WorkflowData> execute(
                         }
                     }, ex -> {
                         Exception e = getSafeException(ex);
-                        String errorMessage = (e == null ? "Failed to update the index settings for index " + indexName : e.getMessage());
+                        String errorMessage = (e == null ? "Failed to retrieve the index settings for index " + indexName : e.getMessage());
+                        logger.error(errorMessage, e);
                         updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
                     }));