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 9, 2024
1 parent 70e0d4e commit caa0381
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 140 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';
import { MR_CHARACTER_LIMIT, SubmitLabel } 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 @@ -110,13 +128,15 @@ const RegisterModel: React.FC = () => {
</StackItem>
</Stack>
<RegistrationFormFooter
submitLabel="Register model"
submitLabel={SubmitLabel.REGISTER_MODEL}
submitError={submitError}
setSubmitError={setSubmitError}
isSubmitDisabled={isSubmitDisabled}
isSubmitting={isSubmitting}
onSubmit={onSubmit}
onSubmit={handleSubmit}
onCancel={onCancel}
errorName={errorName}
versionName={versionName}
modelName={registeredModelName}
/>
</Form>
</PageSection>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import React from 'react';
import { Alert, AlertActionCloseButton, StackItem } from '@patternfly/react-core';
import { ErrorName, SubmitLabel } from './const';

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 === SubmitLabel.REGISTER_MODEL && errorName === ErrorName.MODEL_VERSION) {
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 === SubmitLabel.REGISTER_VERSION && errorName === ErrorName.MODEL_VERSION) {
return (
<StackItem>
<Alert isInline variant="danger" title={`Failed to register ${versionName} version`}>
{submitError.message}
</Alert>
</StackItem>
);
}

if (submitLabel === SubmitLabel.REGISTER_MODEL && errorName === ErrorName.MODEL_ARTIFACT) {
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 === SubmitLabel.REGISTER_VERSION && errorName === ErrorName.MODEL_ARTIFACT) {
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,29 @@ 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';
import { SubmitLabel } from './const';

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 +52,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 @@ -125,13 +143,14 @@ const RegisterVersion: React.FC = () => {
</StackItem>
</Stack>
<RegistrationFormFooter
submitLabel="Register new version"
submitLabel={SubmitLabel.REGISTER_VERSION}
errorName={errorName}
submitError={submitError}
setSubmitError={setSubmitError}
isSubmitDisabled={isSubmitDisabled}
isSubmitting={isSubmitting}
onSubmit={onSubmit}
onSubmit={handleSubmit}
onCancel={onCancel}
versionName={versionName}
/>
</Form>
</PageSection>
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 hasBodyWrapper={false} stickyOnBreakpoint={{ default: 'bottom' }}>
<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
11 changes: 11 additions & 0 deletions frontend/src/pages/modelRegistry/screens/RegisterModel/const.ts
Original file line number Diff line number Diff line change
@@ -1 +1,12 @@
export const MR_CHARACTER_LIMIT = 128;

export enum SubmitLabel {
REGISTER_MODEL = 'Register model',
REGISTER_VERSION = 'Register new version',
}

export enum ErrorName {
REGISTERED_MODEL = 'registeredModel',
MODEL_VERSION = 'modelVersion',
MODEL_ARTIFACT = 'modelArtifact',
}

This file was deleted.

Loading

0 comments on commit caa0381

Please sign in to comment.