Skip to content

Commit

Permalink
Refactor model registry routes and configuration in ModelRegistryCore…
Browse files Browse the repository at this point in the history
…Loader.tsx, ModelRegistryRoutes.tsx, and InvalidModelRegistry.tsx
  • Loading branch information
lucferbux committed Apr 26, 2024
1 parent 5499266 commit 9350de4
Show file tree
Hide file tree
Showing 13 changed files with 276 additions and 92 deletions.
46 changes: 22 additions & 24 deletions backend/src/routes/api/service/modelregistry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,29 @@ import { DEV_MODE } from '../../../../utils/constants';
import { getParam, setParam } from '../../../../utils/proxy';

export default async (fastify: KubeFastifyInstance): Promise<void> => {
if (DEV_MODE) {
fastify.register(httpProxy, {
upstream: '',
prefix: '/:name',
rewritePrefix: '',
replyOptions: {
// preHandler must set the `upstream` param
getUpstream: (request) => getParam(request, 'upstream'),
},
preHandler: (request, _, done) => {
const name = getParam(request, 'name');
fastify.register(httpProxy, {
upstream: '',
prefix: '/:name',
rewritePrefix: '',
replyOptions: {
// preHandler must set the `upstream` param
getUpstream: (request) => getParam(request, 'upstream'),
},
preHandler: (request, _, done) => {
const name = getParam(request, 'name');

const upstream = DEV_MODE
? // Use port forwarding for local development:
// kubectl port-forward -n <namespace> svc/<service-name> <local.port>:<service.port>
`http://${process.env.MODEL_REGISTRY_SERVICE_HOST}:${process.env.MODEL_REGISTRY_SERVICE_PORT}`
: // Construct service URL
`http://${name}.odh-model-registries.svc.cluster.local:8080`;
const upstream = DEV_MODE
? // Use port forwarding for local development:
// kubectl port-forward -n <namespace> svc/<service-name> <local.port>:<service.port>
`http://${process.env.MODEL_REGISTRY_SERVICE_HOST}:${process.env.MODEL_REGISTRY_SERVICE_PORT}`
: // Construct service URL
`http://${name}.odh-model-registries.svc.cluster.local:8080`;

// assign the `upstream` param so we can dynamically set the upstream URL for http-proxy
setParam(request, 'upstream', upstream);
// assign the `upstream` param so we can dynamically set the upstream URL for http-proxy
setParam(request, 'upstream', upstream);

fastify.log.info(`Proxy ${request.method} request ${request.url} to ${upstream}`);
done();
},
});
}
fastify.log.info(`Proxy ${request.method} request ${request.url} to ${upstream}`);
done();
},
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('Model Registry', () => {

modelRegistry.visit();
modelRegistry.navigate();
modelRegistry.shouldtableToolbarExist();
modelRegistry.shouldModelRegistrySelectorExist();
modelRegistry.shouldregisteredModelsEmpty();
});

Expand Down
4 changes: 4 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ class ModelRegistry {
cy.findByTestId('no-registered-models').should('exist');
}

shouldModelRegistrySelectorExist() {
cy.get('#model-registry-selector-dropdown').should('exist');
}

shouldtableToolbarExist() {
cy.findByTestId('registered-models-table-toolbar').should('exist');
}
Expand Down
1 change: 0 additions & 1 deletion frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { useApplicationSettings } from './useApplicationSettings';
import TelemetrySetup from './TelemetrySetup';
import { logout } from './appUtils';
import QuickStarts from './QuickStarts';

import './App.scss';

const App: React.FC = () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import * as React from 'react';
import { Alert, Bullseye } from '@patternfly/react-core';
import { SupportedArea, conditionalArea } from '~/concepts/areas';
import { ModelRegistryKind } from '~/k8sTypes';
import useModelRegistries from '~/concepts/modelRegistry/apiHooks/useModelRegistries';
import { MODEL_REGISTRY_DEFAULT_NAMESPACE } from '~/concepts/modelRegistry/const';
import useModelRegistryAPIState, { ModelRegistryAPIState } from './useModelRegistryAPIState';
import {
Expand All @@ -19,9 +17,6 @@ export type ModelRegistryContextType = {
ignoreTimedOut: () => void;
refreshState: () => Promise<undefined>;
refreshAPIState: () => void;
modelRegistries: ModelRegistryKind[];
preferredModelRegistry: ModelRegistryKind | undefined;
updatePreferredModelRegistry: (modelRegistry: ModelRegistryKind | undefined) => void;
};

type ModelRegistryContextProviderProps = {
Expand All @@ -37,25 +32,18 @@ export const ModelRegistryContext = React.createContext<ModelRegistryContextType
ignoreTimedOut: () => undefined,
refreshState: async () => undefined,
refreshAPIState: () => undefined,
modelRegistries: [],
preferredModelRegistry: undefined,
updatePreferredModelRegistry: () => undefined,
});

export const ModelRegistryContextProvider = conditionalArea<ModelRegistryContextProviderProps>(
SupportedArea.MODEL_REGISTRY,
true,
)(({ children, modelRegistryName }) => {
const [modelRegistries] = useModelRegistries();
const [preferredModelRegistry, setPreferredModelRegistry] =
React.useState<ModelRegistryContextType['preferredModelRegistry']>(undefined);

const crState = useModelRegistryNamespaceCR(MODEL_REGISTRY_DEFAULT_NAMESPACE, modelRegistryName);
const [modelRegistryNamespaceCR, crLoaded, crLoadError, refreshCR] = crState;
const isCRReady = isModelRegistryAvailable(crState);
const state = useModelRegistryNamespaceCR(MODEL_REGISTRY_DEFAULT_NAMESPACE, modelRegistryName);
const [modelRegistryCR, crLoaded, crLoadError, refreshCR] = state;
const isCRReady = isModelRegistryAvailable(state);

const [disableTimeout, setDisableTimeout] = React.useState(false);
const serverTimedOut = !disableTimeout && hasServerTimedOut(crState, isCRReady);
const serverTimedOut = !disableTimeout && hasServerTimedOut(state, isCRReady);
const ignoreTimedOut = React.useCallback(() => {
setDisableTimeout(true);
}, []);
Expand All @@ -64,29 +52,16 @@ export const ModelRegistryContextProvider = conditionalArea<ModelRegistryContext

const [apiState, refreshAPIState] = useModelRegistryAPIState(hostPath);

React.useEffect(() => {
if (modelRegistries.length > 0 && !preferredModelRegistry) {
setPreferredModelRegistry(modelRegistries[0]);
}
}, [modelRegistries, preferredModelRegistry]);

const refreshState = React.useCallback(
() => Promise.all([refreshCR()]).then(() => undefined),
[refreshCR],
);

const updatePreferredModelRegistry = React.useCallback<
ModelRegistryContextType['updatePreferredModelRegistry']
>((modelRegistry) => {
setPreferredModelRegistry(modelRegistry);
}, []);

const error = crLoadError;
if (error) {
if (crLoadError) {
return (
<Bullseye>
<Alert title="Model registry load error" variant="danger" isInline>
{error.message}
{crLoadError.message}
</Alert>
</Bullseye>
);
Expand All @@ -95,16 +70,13 @@ export const ModelRegistryContextProvider = conditionalArea<ModelRegistryContext
return (
<ModelRegistryContext.Provider
value={{
hasCR: !!modelRegistryNamespaceCR,
hasCR: !!modelRegistryCR,
crInitializing: !crLoaded,
serverTimedOut,
apiState,
ignoreTimedOut,
refreshState,
refreshAPIState,
modelRegistries,
preferredModelRegistry,
updatePreferredModelRegistry,
}}
>
{children}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import * as React from 'react';
import { Alert, Bullseye } from '@patternfly/react-core';
import { SupportedArea, conditionalArea } from '~/concepts/areas';
import { ModelRegistryKind } from '~/k8sTypes';
import useModelRegistries from '~/concepts/modelRegistry/apiHooks/useModelRegistries';

export type ModelRegistrySelectorContextType = {
modelRegistries: ModelRegistryKind[];
preferredModelRegistry: ModelRegistryKind | undefined;
updatePreferredModelRegistry: (modelRegistry: ModelRegistryKind | undefined) => void;
};

type ModelRegistrySelectorContextProviderProps = {
children: React.ReactNode;
};

export const ModelRegistrySelectorContext = React.createContext<ModelRegistrySelectorContextType>({
modelRegistries: [],
preferredModelRegistry: undefined,
updatePreferredModelRegistry: () => undefined,
});

export const ModelRegistrySelectorContextProvider =
conditionalArea<ModelRegistrySelectorContextProviderProps>(
SupportedArea.MODEL_REGISTRY,
true,
)(({ children }) => {
const [modelRegistries, isLoaded, error] = useModelRegistries();
const [preferredModelRegistry, setPreferredModelRegistry] =
React.useState<ModelRegistrySelectorContextType['preferredModelRegistry']>(undefined);

const firstModelRegistry = modelRegistries.length > 0 ? modelRegistries[0] : null;

React.useEffect(() => {
if (firstModelRegistry && !preferredModelRegistry) {
setPreferredModelRegistry(firstModelRegistry);
}
}, [firstModelRegistry, preferredModelRegistry]);

const updatePreferredModelRegistry = React.useCallback<
ModelRegistrySelectorContextType['updatePreferredModelRegistry']
>((modelRegistry) => {
setPreferredModelRegistry(modelRegistry);
}, []);

if (!isLoaded) {
return <Bullseye>Loading model registries...</Bullseye>;
}

if (error) {
return (
<Bullseye>
<Alert title="Model registry load error" variant="danger" isInline>
{error.message}
</Alert>
</Bullseye>
);
}

return (
<ModelRegistrySelectorContext.Provider
value={{
modelRegistries,
preferredModelRegistry,
updatePreferredModelRegistry,
}}
>
{children}
</ModelRegistrySelectorContext.Provider>
);
});
79 changes: 72 additions & 7 deletions frontend/src/pages/modelRegistry/ModelRegistryCoreLoader.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,77 @@
import * as React from 'react';
import { Outlet } from 'react-router';

import { Navigate, Outlet, useParams } from 'react-router';
import { ModelRegistryContextProvider } from '~/concepts/modelRegistry/context/ModelRegistryContext';
import ApplicationsPage from '~/pages/ApplicationsPage';
import TitleWithIcon from '~/concepts/design/TitleWithIcon';
import { ProjectObjectType } from '~/concepts/design/utils';

import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext';
import InvalidModelRegistry from './screens/InvalidModelRegistry';
import ModelRegistrySelector from './screens/ModelRegistrySelector';
import EmptyRegisteredModels from './screens/EmptyRegisteredModels';

type ApplicationPageProps = React.ComponentProps<typeof ApplicationsPage>;
type EmptyStateProps = 'emptyStatePage' | 'empty';

type ModelRegistryCoreLoaderProps = {
getInvalidRedirectPath: (modelRegistry: string) => string;
};

type ApplicationPageRenderState = Pick<ApplicationPageProps, EmptyStateProps>;

// TODO: Parametrize this to make the route dynamic
const ModelRegistryCoreLoader: React.FC = () => (
<ModelRegistryContextProvider modelRegistryName="modelregistry-sample">
<Outlet />
</ModelRegistryContextProvider>
);
const ModelRegistryCoreLoader: React.FC<ModelRegistryCoreLoaderProps> = ({
getInvalidRedirectPath,
}) => {
const { modelRegistry } = useParams<{ modelRegistry: string }>();
const { modelRegistries, preferredModelRegistry } = React.useContext(
ModelRegistrySelectorContext,
);

let renderStateProps: ApplicationPageRenderState & { children?: React.ReactNode };
if (modelRegistries.length === 0) {
renderStateProps = {
empty: true,
emptyStatePage: <EmptyRegisteredModels preferredModelRegistry="TODO: Change" />,
};
} else if (modelRegistry) {
const foundModelRegistry = modelRegistries.find((mr) => mr.metadata.name === modelRegistry);
if (foundModelRegistry) {
// Render the content
return (
<ModelRegistryContextProvider modelRegistryName={modelRegistry}>
<Outlet />
</ModelRegistryContextProvider>
);
}

// They ended up on a non-valid project path
renderStateProps = {
empty: true,
emptyStatePage: (
<InvalidModelRegistry
modelRegistry={modelRegistry}
getRedirectPath={getInvalidRedirectPath}
/>
),
};
} else {
// Redirect the namespace suffix into the URL
const redirectModelRegistry = preferredModelRegistry ?? modelRegistries[0];
return <Navigate to={getInvalidRedirectPath(redirectModelRegistry.metadata.name)} replace />;
}

return (
<ApplicationsPage
title={
<TitleWithIcon title="Registered models" objectType={ProjectObjectType.registeredModels} />
}
{...renderStateProps}
loaded
headerContent={<ModelRegistrySelector />}
provideChildrenPadding
/>
);
};

export default ModelRegistryCoreLoader;
25 changes: 17 additions & 8 deletions frontend/src/pages/modelRegistry/ModelRegistryRoutes.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
import * as React from 'react';
import { Navigate, Route } from 'react-router-dom';
import ProjectsRoutes from '~/concepts/projects/ProjectsRoutes';
import { Navigate, Route, Routes } from 'react-router-dom';
import { ModelRegistrySelectorContextProvider } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext';
import ModelRegistryCoreLoader from './ModelRegistryCoreLoader';
import ModelRegistry from './screens/ModelRegistry';

const ModelServingRoutes: React.FC = () => (
<ProjectsRoutes>
<Route path={'/:modelRegistry?/*'} element={<ModelRegistryCoreLoader />}>
<Route index element={<ModelRegistry />} />
<Route path="*" element={<Navigate to="." />} />
</Route>
</ProjectsRoutes>
<ModelRegistrySelectorContextProvider>
<Routes>
<Route
path={'/:modelRegistry?/*'}
element={
<ModelRegistryCoreLoader
getInvalidRedirectPath={(modelRegistry) => `/modelRegistry/${modelRegistry}`}
/>
}
>
<Route index element={<ModelRegistry />} />
<Route path="*" element={<Navigate to="." />} />
</Route>
</Routes>
</ModelRegistrySelectorContextProvider>
);

export default ModelServingRoutes;
14 changes: 14 additions & 0 deletions frontend/src/pages/modelRegistry/screens/EmptyRegisteredModels.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
Button,
EmptyState,
EmptyStateBody,
EmptyStateFooter,
EmptyStateHeader,
EmptyStateIcon,
EmptyStateVariant,
Expand All @@ -22,6 +24,18 @@ const EmptyRegisteredModels: React.FC<EmptyRegisteredModelsType> = ({ preferredM
<br />
registry or select a different one.
</EmptyStateBody>
<EmptyStateFooter>
<Button
id="register-model-empty-button"
key="register-model-empty-button"
data-testid="register-model-empty-button"
aria-label="Register model"
onClick={() => undefined}
>
Register model
</Button>
,
</EmptyStateFooter>
</EmptyState>
);

Expand Down
Loading

0 comments on commit 9350de4

Please sign in to comment.