Skip to content

Commit

Permalink
Model Details and Model Version Details: add editable properties sect…
Browse files Browse the repository at this point in the history 
…ion, set up API patches for description, labels and properties

Signed-off-by: Mike Turley <[email protected]>
  • Loading branch information
@mturley
mturley committed May 8, 2024
1 parent 070fe7e commit e136ed3
Show file tree
Hide file tree
Showing 17 changed files with 983 additions and 180 deletions.
44 changes: 22 additions & 22 deletions frontend/src/__mocks__/mockRegisteredModelsList.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable camelcase */
import { RegisteredModelList } from '~/concepts/modelRegistry/types';
import { ModelRegistryMetadataType, RegisteredModelList } from '~/concepts/modelRegistry/types';
import { mockRegisteredModel } from './mockRegisteredModel';

export const mockRegisteredModelList = ({
Expand All @@ -14,27 +14,27 @@ export const mockRegisteredModelList = ({
'A machine learning model trained to detect fraudulent transactions in financial data',
customProperties: {
Financial: {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: 'non-empty',
},
'Financial data': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Fraud detection': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Test label': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Machine learning': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Next data to be overflow': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand All @@ -43,23 +43,23 @@ export const mockRegisteredModelList = ({
name: 'Credit Scoring',
customProperties: {
'Credit Score Predictor': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Creditworthiness scoring system': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Default Risk Analyzer': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Portfolio Management': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Risk Assessment': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand All @@ -70,44 +70,44 @@ export const mockRegisteredModelList = ({
'A machine learning model trained to detect fraudulent transactions in financial data',
customProperties: {
'Testing label': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
Financial: {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: 'non-empty',
},
'Financial data': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Fraud detection': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc':
{
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Machine learning': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Next data to be overflow': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label x': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label y': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
'Label z': {
metadataType: 'MetadataStringValue',
metadataType: ModelRegistryMetadataType.STRING,
string_value: '',
},
},
Expand Down
11 changes: 2 additions & 9 deletions frontend/src/components/DashboardDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
DescriptionListTerm,
Split,
SplitItem,
Text,
} from '@patternfly/react-core';
import text from '@patternfly/react-styles/css/utilities/Text/text';
import { CheckIcon, PencilAltIcon, TimesIcon } from '@patternfly/react-icons';
Expand Down Expand Up @@ -70,7 +69,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
</ActionListItem>
<ActionListItem>
<Button
data-testid={`discard-edit-button-${title} `}
data-testid={`discard-edit-button-${title}`}
aria-label={`Discard edits to ${title} `}
variant="plain"
onClick={onDiscardEditsClick}
Expand Down Expand Up @@ -100,13 +99,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
<DescriptionListTerm>{title}</DescriptionListTerm>
)}
<DescriptionListDescription className={isEmpty && !isEditing ? text.disabledColor_100 : ''}>
{isEditing ? (
contentWhenEditing
) : isEmpty ? (
<Text style={{ color: '--pf-v5-global--Color--200' }}>{contentWhenEmpty}</Text>
) : (
children
)}
{isEditing ? contentWhenEditing : isEmpty ? contentWhenEmpty : children}
</DescriptionListDescription>
</DescriptionListGroup>
);
Expand Down
33 changes: 26 additions & 7 deletions frontend/src/components/EditableLabelsDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ type EditableTextDescriptionListGroupProps = Partial<
Pick<DashboardDescriptionListGroupProps, 'title' | 'contentWhenEmpty'>
> & {
labels: string[];
saveEditedLabels: (labels: string[]) => Promise<void>;
saveEditedLabels: (labels: string[]) => Promise<unknown>;
allExistingKeys?: string[];
};

const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
title = 'Labels',
contentWhenEmpty = 'No labels',
labels,
saveEditedLabels,
allExistingKeys = labels,
}) => {
const [isEditing, setIsEditing] = React.useState(false);
const [unsavedLabels, setUnsavedLabels] = React.useState(labels);
Expand All @@ -54,10 +56,22 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
setUnsavedLabels([...unsavedLabels, text]);
};

// Don't allow a label that matches a non-label property key or another label (as they stand before saving)
// Note that this means if you remove a label and add it back before saving, that is valid
const reservedKeys = [
...allExistingKeys.filter((key) => !labels.includes(key)),
...unsavedLabels,
];

const [isAddLabelModalOpen, setIsAddLabelModalOpen] = React.useState(false);
const [addLabelInputValue, setAddLabelInputValue] = React.useState('');
const addLabelInputRef = React.useRef<HTMLInputElement>(null);
const addLabelInputTooLong = addLabelInputValue.length > 63;
let addLabelValidationError: string | null = null;
if (reservedKeys.includes(addLabelInputValue)) {
addLabelValidationError = 'Label must not match an existing label or property key';
} else if (addLabelInputValue.length > 63) {
addLabelValidationError = "Label text can't exceed 63 characters";
}

const toggleAddLabelModal = () => {
setAddLabelInputValue('');
Expand All @@ -68,7 +82,8 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
addLabelInputRef.current.focus();
}
}, [isAddLabelModalOpen]);
const addLabelModalSubmitDisabled = !addLabelInputValue || addLabelInputTooLong;

const addLabelModalSubmitDisabled = !addLabelInputValue || !!addLabelValidationError;
const submitAddLabelModal = (event?: React.FormEvent) => {
event?.preventDefault();
if (!addLabelModalSubmitDisabled) {
Expand Down Expand Up @@ -107,7 +122,11 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
editableProps={{ 'aria-label': `Editable label with text ${label}` }}
onClose={() => removeUnsavedLabel(label)}
closeBtnProps={{ isDisabled: isSavingEdits }}
onEditComplete={(_event, newText) => editUnsavedLabel(newText, index)}
onEditComplete={(_event, newText) => {
if (!reservedKeys.includes(newText) && newText.length <= 63) {
editUnsavedLabel(newText, index);
}
}}
>
{label}
</Label>
Expand Down Expand Up @@ -172,13 +191,13 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
}
ref={addLabelInputRef}
isRequired
validated={addLabelInputTooLong ? 'error' : 'default'}
validated={addLabelValidationError ? 'error' : 'default'}
/>
{addLabelInputTooLong && (
{addLabelValidationError && (
<FormHelperText>
<HelperText>
<HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
Label text can&apos;t exceed 63 characters
{addLabelValidationError}
</HelperTextItem>
</HelperText>
</FormHelperText>
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/EditableTextDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type EditableTextDescriptionListGroupProps = Pick<
'title' | 'contentWhenEmpty'
> & {
value: string;
saveEditedValue: (value: string) => Promise<void>;
saveEditedValue: (value: string) => Promise<unknown>;
};

const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
Expand Down Expand Up @@ -37,6 +37,8 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
value={unsavedValue}
onChange={(_event, v) => setUnsavedValue(v)}
isDisabled={isSavingEdits}
rows={24}
resizeOrientation="vertical"
/>
}
onEditClick={() => {
Expand Down
53 changes: 52 additions & 1 deletion frontend/src/concepts/modelRegistry/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,65 @@ export enum ModelArtifactState {
REFERENCE = 'REFERENCE',
}

export enum ModelRegistryMetadataType {
INT = 'MetadataIntValue',
DOUBLE = 'MetadataDoubleValue',
STRING = 'MetadataStringValue',
STRUCT = 'MetadataStructValue',
PROTO = 'MetadataProtoValue',
BOOL = 'MetadataBoolValue',
}

export type ModelRegistryCustomPropertyInt = {
metadataType: ModelRegistryMetadataType.INT;
int_value: string; // int64-formatted string
};

export type ModelRegistryCustomPropertyDouble = {
metadataType: ModelRegistryMetadataType.DOUBLE;
double_value: number;
};

export type ModelRegistryCustomPropertyString = {
metadataType: ModelRegistryMetadataType.STRING;
string_value: string;
};

export type ModelRegistryCustomPropertyStruct = {
metadataType: ModelRegistryMetadataType.STRUCT;
struct_value: string; // Base64 encoded bytes for struct value
};

export type ModelRegistryCustomPropertyProto = {
metadataType: ModelRegistryMetadataType.PROTO;
type: string; // url describing proto value
proto_value: string; // Base64 encoded bytes for proto value
};

export type ModelRegistryCustomPropertyBool = {
metadataType: ModelRegistryMetadataType.BOOL;
bool_value: boolean;
};

export type ModelRegistryCustomProperty =
| ModelRegistryCustomPropertyInt
| ModelRegistryCustomPropertyDouble
| ModelRegistryCustomPropertyString
| ModelRegistryCustomPropertyStruct
| ModelRegistryCustomPropertyProto
| ModelRegistryCustomPropertyBool;

export type ModelRegistryCustomProperties = Record<string, ModelRegistryCustomProperty>;
export type ModelRegistryStringCustomProperties = Record<string, ModelRegistryCustomPropertyString>;

export type ModelRegistryBase = {
id: string;
name: string;
externalID?: string;
description?: string;
createTimeSinceEpoch?: string;
lastUpdateTimeSinceEpoch: string;
customProperties: Record<string, Record<string, string>>;
customProperties: ModelRegistryCustomProperties;
};

export type ModelArtifact = ModelRegistryBase & {
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/pages/modelRegistry/screens/GlobalModelVersionsTabs.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ type GlobalModelVersionsTabProps = {
tab: ModelVersionsTabs;
registeredModel: RegisteredModel;
modelVersions: ModelVersion[];
refresh: () => void;
};

const GlobalModelVersionsTabs: React.FC<GlobalModelVersionsTabProps> = ({
tab,
registeredModel: rm,
modelVersions,
refresh,
}) => {
const navigate = useNavigate();
return (
Expand Down Expand Up @@ -44,7 +46,7 @@ const GlobalModelVersionsTabs: React.FC<GlobalModelVersionsTabProps> = ({
data-testid="model-details-tab"
>
<PageSection isFilled variant="light" data-testid="model-details-tab-content">
<ModelDetailsView registeredModel={rm} />
<ModelDetailsView registeredModel={rm} refresh={refresh} />
</PageSection>
</Tab>
</Tabs>
Expand Down
Loading

0 comments on commit e136ed3

Please sign in to comment.