Refactor quick configure components; improve processor error handling (…

…#604) (#609) (cherry picked from commit ea7d2c0) Signed-off-by: Tyler Ohlsen <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Feb 10, 2025 · 5d13c93 · 5d13c93
1 parent 4e46cf3
commit 5d13c93
Show file tree

Hide file tree

Showing 15 changed files with 793 additions and 788 deletions.
diff --git a/public/pages/workflow_detail/components/edit_workflow_metadata_modal.tsx b/public/pages/workflow_detail/components/edit_workflow_metadata_modal.tsx
@@ -85,7 +85,7 @@ export function EditWorkflowMetadataModal(
         }
       )
       .required('Required') as yup.Schema,
-    desription: yup
+    description: yup
       .string()
       .min(0)
       .max(MAX_DESCRIPTION_LENGTH, 'Too long')

diff --git a/public/pages/workflow_detail/tools/errors/errors.tsx b/public/pages/workflow_detail/tools/errors/errors.tsx
@@ -4,11 +4,15 @@
  */
 
 import React from 'react';
-import { EuiCodeBlock, EuiEmptyPrompt } from '@elastic/eui';
-import { isEmpty } from 'lodash';
+import {
+  EuiCodeBlock,
+  EuiEmptyPrompt,
+  EuiFlexItem,
+  EuiSpacer,
+} from '@elastic/eui';
 
 interface ErrorsProps {
-  errorMessage: string;
+  errorMessages: string[];
 }
 
 /**
@@ -18,12 +22,21 @@ interface ErrorsProps {
 export function Errors(props: ErrorsProps) {
   return (
     <>
-      {isEmpty(props.errorMessage) ? (
+      {props.errorMessages?.length === 0 ? (
         <EuiEmptyPrompt title={<h2>No errors</h2>} titleSize="s" />
       ) : (
-        <EuiCodeBlock fontSize="m" isCopyable={false}>
-          {props.errorMessage}
-        </EuiCodeBlock>
+        <>
+          {props.errorMessages.map((errorMessage, idx) => {
+            return (
+              <EuiFlexItem grow={false} key={idx}>
+                <EuiSpacer size="m" />
+                <EuiCodeBlock fontSize="m" isCopyable={false} paddingSize="s">
+                  {errorMessage}
+                </EuiCodeBlock>
+              </EuiFlexItem>
+            );
+          })}
+        </>
       )}
     </>
   );

diff --git a/public/pages/workflow_detail/tools/query/query.tsx b/public/pages/workflow_detail/tools/query/query.tsx
@@ -29,14 +29,12 @@ import {
 import {
   AppState,
   searchIndex,
-  setOpenSearchError,
   setSearchPipelineErrors,
   useAppDispatch,
 } from '../../../../store';
 import {
   containsEmptyValues,
   containsSameValues,
-  formatSearchPipelineErrors,
   getDataSourceId,
   getPlaceholdersFromQuery,
   getSearchPipelineErrors,
@@ -225,15 +223,6 @@ export function Query(props: QueryProps) {
                                     errors: searchPipelineErrors,
                                   })
                                 );
-                                if (!isEmpty(searchPipelineErrors)) {
-                                  dispatch(
-                                    setOpenSearchError({
-                                      error: `Error running search pipeline. ${formatSearchPipelineErrors(
-                                        searchPipelineErrors
-                                      )}`,
-                                    })
-                                  );
-                                }
                               } else {
                                 setSearchPipelineErrors({ errors: {} });
                               }

diff --git a/public/pages/workflow_detail/tools/tools.tsx b/public/pages/workflow_detail/tools/tools.tsx
@@ -44,35 +44,55 @@ const PANEL_TITLE = 'Inspect flows';
  * The base Tools component for performing ingest and search, viewing resources, and debugging.
  */
 export function Tools(props: ToolsProps) {
-  // error message state
+  // error message states. Error may come from several different sources.
   const { opensearch, workflows } = useSelector((state: AppState) => state);
   const opensearchError = opensearch.errorMessage;
   const workflowsError = workflows.errorMessage;
-  const [curErrorMessage, setCurErrorMessage] = useState<string>('');
+  const {
+    ingestPipeline: ingestPipelineErrors,
+    searchPipeline: searchPipelineErrors,
+  } = useSelector((state: AppState) => state.errors);
+  const [curErrorMessages, setCurErrorMessages] = useState<string[]>([]);
 
-  // auto-navigate to errors tab if a new error has been set as a result of
-  // executing OpenSearch or Flow Framework workflow APIs, or from the workflow state
-  // (note that if provision/deprovision fails, there is no concrete exception returned at the API level -
-  // it is just set in the workflow's error field when fetching workflow state)
+  // Propagate any errors coming from opensearch API calls, including ingest/search pipeline verbose calls.
   useEffect(() => {
-    setCurErrorMessage(opensearchError);
-    if (!isEmpty(opensearchError)) {
-      props.setSelectedTabId(INSPECTOR_TAB_ID.ERRORS);
+    if (
+      !isEmpty(opensearchError) ||
+      !isEmpty(ingestPipelineErrors) ||
+      !isEmpty(searchPipelineErrors)
+    ) {
+      if (!isEmpty(opensearchError)) {
+        setCurErrorMessages([opensearchError]);
+      } else if (!isEmpty(ingestPipelineErrors)) {
+        setCurErrorMessages([
+          'Data not ingested. Errors found with the following ingest processor(s):',
+          ...Object.values(ingestPipelineErrors).map((value) => value.errorMsg),
+        ]);
+      } else if (!isEmpty(searchPipelineErrors)) {
+        setCurErrorMessages([
+          'Errors found with the following search processor(s)',
+          ...Object.values(searchPipelineErrors).map((value) => value.errorMsg),
+        ]);
+      }
+    } else {
+      setCurErrorMessages([]);
     }
-  }, [opensearchError]);
+  }, [opensearchError, ingestPipelineErrors, searchPipelineErrors]);
 
+  // Propagate any errors coming from the workflow, either runtime from API call, or persisted in the indexed workflow itself.
   useEffect(() => {
-    setCurErrorMessage(workflowsError);
-    if (!isEmpty(workflowsError)) {
-      props.setSelectedTabId(INSPECTOR_TAB_ID.ERRORS);
-    }
+    setCurErrorMessages(!isEmpty(workflowsError) ? [workflowsError] : []);
   }, [workflowsError]);
   useEffect(() => {
-    setCurErrorMessage(props.workflow?.error || '');
-    if (!isEmpty(props.workflow?.error)) {
+    setCurErrorMessages(props.workflow?.error ? [props.workflow.error] : []);
+  }, [props.workflow?.error]);
+
+  // auto-navigate to errors tab if new errors have been found
+  useEffect(() => {
+    if (curErrorMessages.length > 0) {
       props.setSelectedTabId(INSPECTOR_TAB_ID.ERRORS);
     }
-  }, [props.workflow?.error]);
+  }, [curErrorMessages]);
 
   // auto-navigate to ingest tab if a populated value has been set, indicating ingest has been ran
   useEffect(() => {
@@ -136,7 +156,7 @@ export function Tools(props: ToolsProps) {
                   />
                 )}
                 {props.selectedTabId === INSPECTOR_TAB_ID.ERRORS && (
-                  <Errors errorMessage={curErrorMessage} />
+                  <Errors errorMessages={curErrorMessages} />
                 )}
                 {props.selectedTabId === INSPECTOR_TAB_ID.RESOURCES && (
                   <Resources workflow={props.workflow} />

diff --git a/public/pages/workflow_detail/workflow_inputs/config_field_list.tsx b/public/pages/workflow_detail/workflow_inputs/config_field_list.tsx
@@ -126,7 +126,6 @@ export function ConfigFieldList(props: ConfigFieldListProps) {
             el = (
               <EuiFlexItem key={idx}>
                 <ModelField
-                  field={field}
                   fieldPath={fieldPath}
                   showMissingInterfaceCallout={false}
                 />

diff --git a/public/pages/workflow_detail/workflow_inputs/index.ts b/public/pages/workflow_detail/workflow_inputs/index.ts
@@ -4,3 +4,4 @@
  */
 
 export * from './workflow_inputs';
+export * from './input_fields';
diff --git a/public/pages/workflow_detail/workflow_inputs/input_fields/model_field.tsx b/public/pages/workflow_detail/workflow_inputs/input_fields/model_field.tsx
@@ -21,18 +21,21 @@ import {
   MODEL_STATE,
   WorkflowFormValues,
   ModelFormValue,
-  IConfigField,
   ML_CHOOSE_MODEL_LINK,
   ML_REMOTE_MODEL_LINK,
 } from '../../../../../common';
 import { AppState } from '../../../../store';
 
 interface ModelFieldProps {
-  field: IConfigField;
   fieldPath: string; // the full path in string-form to the field (e.g., 'ingest.enrich.processors.text_embedding_processor.inputField')
   hasModelInterface?: boolean;
   onModelChange?: (modelId: string) => void;
   showMissingInterfaceCallout?: boolean;
+  label?: string;
+  helpText?: string;
+  fullWidth?: boolean;
+  showError?: boolean;
+  showInvalid?: boolean;
 }
 
 type ModelItem = ModelFormValue & {
@@ -118,19 +121,28 @@ export function ModelField(props: ModelFieldProps) {
       )}
       <Field name={props.fieldPath}>
         {({ field, form }: FieldProps) => {
+          const isInvalid =
+            (props.showInvalid ?? true) &&
+            getIn(errors, `${field.name}.id`) &&
+            getIn(touched, `${field.name}.id`);
           return (
             <EuiCompressedFormRow
-              label={'Model'}
+              fullWidth={props.fullWidth}
+              label={props.label || 'Model'}
               labelAppend={
                 <EuiText size="xs">
                   <EuiLink href={ML_CHOOSE_MODEL_LINK} target="_blank">
                     Learn more
                   </EuiLink>
                 </EuiText>
               }
-              helpText={'The model ID.'}
+              helpText={props.helpText || 'The model ID.'}
+              isInvalid={isInvalid}
+              error={props.showError && getIn(errors, `${field.name}.id`)}
             >
               <EuiCompressedSuperSelect
+                data-testid="selectDeployedModel"
+                fullWidth={props.fullWidth}
                 disabled={isEmpty(deployedModels)}
                 options={deployedModels.map(
                   (option) =>
@@ -165,11 +177,7 @@ export function ModelField(props: ModelFieldProps) {
                     props.onModelChange(option);
                   }
                 }}
-                isInvalid={
-                  getIn(errors, field.name) && getIn(touched, field.name)
-                    ? true
-                    : undefined
-                }
+                isInvalid={isInvalid}
               />
             </EuiCompressedFormRow>
           );

diff --git a/...kflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs/ml_processor_inputs.tsx b/...kflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs/ml_processor_inputs.tsx
@@ -200,7 +200,6 @@ export function MLProcessorInputs(props: MLProcessorInputsProps) {
         />
       ) : (
         <ModelField
-          field={modelField}
           fieldPath={modelFieldPath}
           hasModelInterface={modelInterface !== undefined}
           onModelChange={onModelChange}

diff --git a/public/pages/workflow_detail/workflow_inputs/processors_list.tsx b/public/pages/workflow_detail/workflow_inputs/processors_list.tsx
@@ -17,7 +17,8 @@ import {
   EuiAccordion,
   EuiSpacer,
   EuiText,
-  EuiIconTip,
+  EuiIcon,
+  EuiCallOut,
 } from '@elastic/eui';
 import { cloneDeep, isEmpty } from 'lodash';
 import { getIn, useFormikContext } from 'formik';
@@ -85,6 +86,11 @@ export function ProcessorsList(props: ProcessorsListProps) {
   const [isPopoverOpen, setPopover] = useState(false);
   const [processors, setProcessors] = useState<IProcessorConfig[]>([]);
 
+  // Accordions do not persist an open/closed state, so we manually persist
+  const [processorOpenState, setProcessorOpenState] = useState<{
+    [processorId: string]: boolean;
+  }>({});
+
   function clearProcessorErrors(): void {
     if (props.context === PROCESSOR_CONTEXT.INGEST) {
       dispatch(setIngestPipelineErrors({ errors: {} }));
@@ -379,31 +385,60 @@ export function ProcessorsList(props: ProcessorsListProps) {
                 `${processorIndex}.errorMsg`,
                 undefined
               );
+        const errorFound =
+          processorFormError !== undefined ||
+          processorRuntimeError !== undefined;
+        const isAddedProcessor =
+          processorAdded && processorIndex === processors.length - 1;
+        const processorOpen =
+          processorOpenState[processor.id] ?? isAddedProcessor;
+        const errorMsg = processorFormError
+          ? 'Form error(s) detected'
+          : 'Runtime error(s) detected';
 
         return (
           <EuiFlexItem key={processorIndex}>
             <EuiPanel paddingSize="s">
               <EuiAccordion
-                initialIsOpen={
-                  processorAdded && processorIndex === processors.length - 1
-                }
+                initialIsOpen={isAddedProcessor}
                 id={`accordion${processor.id}`}
+                onToggle={(isOpen) => {
+                  setProcessorOpenState({
+                    ...processorOpenState,
+                    [processor.id]: isOpen,
+                  });
+                }}
                 buttonContent={
-                  <EuiFlexGroup direction="row">
+                  <EuiFlexGroup direction="column" gutterSize="none">
                     <EuiFlexItem grow={false}>
                       <EuiText>{`${processor.name || 'Processor'}`}</EuiText>
                     </EuiFlexItem>
-                    {(processorFormError !== undefined ||
-                      processorRuntimeError !== undefined) && (
+                    {errorFound && !processorOpen && (
                       <EuiFlexItem grow={false}>
-                        <EuiIconTip
-                          aria-label="Warning"
-                          size="m"
-                          type="alert"
-                          color="danger"
-                          content={processorFormError || processorRuntimeError}
-                          position="right"
-                        />
+                        <EuiFlexGroup
+                          direction="row"
+                          gutterSize="s"
+                          alignItems="flexStart"
+                        >
+                          <EuiFlexItem grow={false}>
+                            <EuiIcon
+                              color="danger"
+                              type={'alert'}
+                              style={{ marginTop: '4px' }}
+                            />
+                          </EuiFlexItem>
+                          <EuiFlexItem grow={false}>
+                            <EuiText
+                              size="s"
+                              color="danger"
+                              style={{
+                                marginTop: '0px',
+                              }}
+                            >
+                              {errorMsg}
+                            </EuiText>
+                          </EuiFlexItem>
+                        </EuiFlexGroup>
                       </EuiFlexItem>
                     )}
                   </EuiFlexGroup>
@@ -421,6 +456,21 @@ export function ProcessorsList(props: ProcessorsListProps) {
               >
                 <EuiSpacer size="s" />
                 <EuiFlexItem style={{ paddingLeft: '28px' }}>
+                  {errorFound && processorOpen && (
+                    <>
+                      <EuiCallOut
+                        size="s"
+                        color="danger"
+                        iconType={'alert'}
+                        title={errorMsg}
+                      >
+                        <EuiText size="s">
+                          {processorFormError || processorRuntimeError}
+                        </EuiText>
+                      </EuiCallOut>
+                      <EuiSpacer size="s" />
+                    </>
+                  )}
                   <ProcessorInputs
                     uiConfig={props.uiConfig}
                     config={processor}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,3 +4,4 @@
		*/

		export * from './workflow_inputs';
		export * from './input_fields';