Skip to content

Commit

Permalink
Improve error message for model registration
Browse files Browse the repository at this point in the history
  • Loading branch information
ppadti committed Dec 3, 2024
1 parent 3ba248d commit d278732
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,49 @@ import { Link } from 'react-router-dom';
import FormSection from '~/components/pf-overrides/FormSection';
import ApplicationsPage from '~/pages/ApplicationsPage';
import { modelRegistryUrl, registeredModelUrl } from '~/pages/modelRegistry/screens/routeUtils';
import { ModelRegistryContext } from '~/concepts/modelRegistry/context/ModelRegistryContext';
import { useAppSelector } from '~/redux/hooks';
import { useRegisterModelData } from './useRegisterModelData';
import { isNameValid, isRegisterModelSubmitDisabled, registerModel } from './utils';
import RegistrationCommonFormSections from './RegistrationCommonFormSections';
import { useRegistrationCommonState } from './useRegistrationCommonState';
import PrefilledModelRegistryField from './PrefilledModelRegistryField';
import RegistrationFormFooter from './RegistrationFormFooter';
import { MR_CHARACTER_LIMIT } from './const';

const RegisterModel: React.FC = () => {
const { modelRegistry: mrName } = useParams();
const navigate = useNavigate();

const { isSubmitting, submitError, setSubmitError, handleSubmit, apiState, author } =
useRegistrationCommonState();

const { apiState } = React.useContext(ModelRegistryContext);
const author = useAppSelector((state) => state.user || '');
const [isSubmitting, setIsSubmitting] = React.useState(false);
const [submitError, setSubmitError] = React.useState<Error | undefined>(undefined);
const [formData, setData] = useRegisterModelData();
const isModelNameValid = isNameValid(formData.modelName);
const isSubmitDisabled = isSubmitting || isRegisterModelSubmitDisabled(formData);
const { modelName, modelDescription } = formData;
const [registeredModelName, setRegisteredModelName] = React.useState<string>('');
const [versionName, setVersionName] = React.useState<string>('');
const [errorName, setErrorName] = React.useState<string | undefined>(undefined);

const handleSubmit = async () => {
setIsSubmitting(true);
setSubmitError(undefined);

const onSubmit = () =>
handleSubmit(async () => {
const { registeredModel } = await registerModel(apiState, formData, author);
const {
data: { registeredModel, modelVersion, modelArtifact },
errors,
} = await registerModel(apiState, formData, author);
if (registeredModel && modelVersion && modelArtifact) {
navigate(registeredModelUrl(registeredModel.id, mrName));
});
} else if (Object.keys(errors).length > 0) {
setIsSubmitting(false);
setRegisteredModelName(formData.modelName);
setVersionName(formData.versionName);
const resourceName = Object.keys(errors)[0];
setErrorName(resourceName);
setSubmitError(errors[resourceName]);
}
};
const onCancel = () => navigate(modelRegistryUrl(mrName));

return (
Expand Down Expand Up @@ -114,11 +132,13 @@ const RegisterModel: React.FC = () => {
<RegistrationFormFooter
submitLabel="Register model"
submitError={submitError}
setSubmitError={setSubmitError}
isSubmitDisabled={isSubmitDisabled}
isSubmitting={isSubmitting}
onSubmit={onSubmit}
onSubmit={handleSubmit}
onCancel={onCancel}
errorName={errorName}
versionName={versionName}
modelName={registeredModelName}
/>
</ApplicationsPage>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import React from 'react';
import { Alert, AlertActionCloseButton, StackItem } from '@patternfly/react-core';

type RegisterModelErrorProp = {
submitLabel: string;
submitError: Error;
errorName?: string;
versionName?: string;
modelName?: string;
};

const RegisterModelErrors: React.FC<RegisterModelErrorProp> = ({
submitLabel,
submitError,
errorName,
versionName,
modelName,
}) => {
const [showAlert, setShowAlert] = React.useState<boolean>(true);

if (submitLabel === 'Register model' && errorName === 'modelVersion') {
return (
<>
{showAlert && (
<StackItem>
<Alert
isInline
variant="success"
title={`${modelName} model registered`}
actionClose={<AlertActionCloseButton onClose={() => setShowAlert(false)} />}
/>
</StackItem>
)}
<StackItem>
<Alert isInline variant="danger" title={`Failed to register ${versionName} version`}>
{submitError.message}
</Alert>
</StackItem>
</>
);
}

if (submitLabel === 'Register new version' && errorName === 'modelVersion') {
return (
<StackItem>
<Alert isInline variant="danger" title={`Failed to register ${versionName} version`}>
{submitError.message}
</Alert>
</StackItem>
);
}

if (submitLabel === 'Register model' && errorName === 'modelArtifact') {
return (
<>
{showAlert && (
<StackItem>
<Alert
isInline
variant="success"
title={`${modelName} model and ${versionName} version registered`}
actionClose={<AlertActionCloseButton onClose={() => setShowAlert(false)} />}
/>
</StackItem>
)}
<StackItem>
<Alert
isInline
variant="danger"
title={`Failed to create artifact for ${versionName} version`}
>
{submitError.message}
</Alert>
</StackItem>
</>
);
}

if (submitLabel === 'Register new version' && errorName === 'modelArtifact') {
return (
<>
{showAlert && (
<StackItem>
<Alert
isInline
variant="success"
title={`${versionName} version registered`}
actionClose={<AlertActionCloseButton onClose={() => setShowAlert(false)} />}
/>
</StackItem>
)}
<StackItem>
<Alert
isInline
variant="danger"
title={`Failed to create artifact for ${versionName} version`}
>
{submitError.message}
</Alert>
</StackItem>
</>
);
}

return (
<StackItem>
<Alert isInline variant="danger" title={`Failed to register ${modelName} model`}>
{submitError.message}
</Alert>
</StackItem>
);
};
export default RegisterModelErrors;
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,28 @@ import ApplicationsPage from '~/pages/ApplicationsPage';
import { modelRegistryUrl, registeredModelUrl } from '~/pages/modelRegistry/screens/routeUtils';
import useRegisteredModels from '~/concepts/modelRegistry/apiHooks/useRegisteredModels';
import { filterLiveModels } from '~/concepts/modelRegistry/utils';
import { ModelRegistryContext } from '~/concepts/modelRegistry/context/ModelRegistryContext';
import { useAppSelector } from '~/redux/hooks';
import { useRegisterVersionData } from './useRegisterModelData';
import { isRegisterVersionSubmitDisabled, registerVersion } from './utils';
import RegistrationCommonFormSections from './RegistrationCommonFormSections';
import { useRegistrationCommonState } from './useRegistrationCommonState';
import PrefilledModelRegistryField from './PrefilledModelRegistryField';
import RegistrationFormFooter from './RegistrationFormFooter';
import RegisteredModelSelector from './RegisteredModelSelector';
import { usePrefillRegisterVersionFields } from './usePrefillRegisterVersionFields';

const RegisterVersion: React.FC = () => {
const { modelRegistry: mrName, registeredModelId: prefilledRegisteredModelId } = useParams();

const navigate = useNavigate();

const { isSubmitting, submitError, setSubmitError, handleSubmit, apiState, author } =
useRegistrationCommonState();

const { apiState } = React.useContext(ModelRegistryContext);
const author = useAppSelector((state) => state.user || '');
const [isSubmitting, setIsSubmitting] = React.useState(false);
const [formData, setData] = useRegisterVersionData(prefilledRegisteredModelId);
const isSubmitDisabled = isSubmitting || isRegisterVersionSubmitDisabled(formData);
const [submitError, setSubmitError] = React.useState<Error | undefined>(undefined);
const [errorName, setErrorName] = React.useState<string | undefined>(undefined);
const [versionName, setVersionName] = React.useState<string>('');

const { registeredModelId } = formData;

const [allRegisteredModels, loadedRegisteredModels, loadRegisteredModelsError] =
Expand All @@ -48,15 +51,29 @@ const RegisterVersion: React.FC = () => {
setData,
});

const onSubmit = () => {
const handleSubmit = async () => {
if (!registeredModel) {
return; // We shouldn't be able to hit this due to form validation
}
handleSubmit(async () => {
await registerVersion(apiState, registeredModel, formData, author);
setIsSubmitting(true);
setSubmitError(undefined);

const {
data: { modelVersion, modelArtifact },
errors,
} = await registerVersion(apiState, registeredModel, formData, author);

if (modelVersion && modelArtifact) {
navigate(registeredModelUrl(registeredModel.id, mrName));
});
} else if (Object.keys(errors).length > 0) {
const resourceName = Object.keys(errors)[0];
setVersionName(formData.versionName);
setErrorName(resourceName);
setSubmitError(errors[resourceName]);
setIsSubmitting(false);
}
};

const onCancel = () =>
navigate(
prefilledRegisteredModelId && registeredModel
Expand Down Expand Up @@ -128,12 +145,13 @@ const RegisterVersion: React.FC = () => {
</PageSection>
<RegistrationFormFooter
submitLabel="Register new version"
errorName={errorName}
submitError={submitError}
setSubmitError={setSubmitError}
isSubmitDisabled={isSubmitDisabled}
isSubmitting={isSubmitting}
onSubmit={onSubmit}
onSubmit={handleSubmit}
onCancel={onCancel}
versionName={versionName}
/>
</ApplicationsPage>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,40 @@
import React from 'react';
import {
PageSection,
Stack,
StackItem,
Alert,
AlertActionCloseButton,
ActionGroup,
Button,
} from '@patternfly/react-core';
import { PageSection, Stack, StackItem, ActionGroup, Button } from '@patternfly/react-core';
import RegisterModelErrors from './RegisterModelErrors';

type RegistrationFormFooterProps = {
submitLabel: string;
submitError?: Error;
setSubmitError: (e?: Error) => void;
isSubmitDisabled: boolean;
isSubmitting: boolean;
onSubmit: () => void;
onCancel: () => void;
errorName?: string;
versionName?: string;
modelName?: string;
};

const RegistrationFormFooter: React.FC<RegistrationFormFooterProps> = ({
submitLabel,
submitError,
setSubmitError,
isSubmitDisabled,
isSubmitting,
onSubmit,
onCancel,
errorName,
versionName,
modelName,
}) => (
<PageSection stickyOnBreakpoint={{ default: 'bottom' }} variant="light">
<Stack hasGutter>
{submitError && (
<StackItem>
<Alert
isInline
variant="danger"
title={submitError.name}
actionClose={<AlertActionCloseButton onClose={() => setSubmitError(undefined)} />}
>
{submitError.message}
</Alert>
</StackItem>
<RegisterModelErrors
submitLabel={submitLabel}
submitError={submitError}
errorName={errorName}
versionName={versionName}
modelName={modelName}
/>
)}
<StackItem>
<ActionGroup>
Expand Down

This file was deleted.

Loading

0 comments on commit d278732

Please sign in to comment.