Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge f/model-serving to main #2123

Merged
merged 70 commits into from
Nov 13, 2023
Merged

Merge f/model-serving to main #2123

merged 70 commits into from
Nov 13, 2023

Conversation

lucferbux
Copy link
Contributor

Description

Closes #1810

DaoDaoNoCode and others added 30 commits September 29, 2023 12:08
Revamp add server button and add tooltip
Show alert when external route is set while token is not set for model server
Improvements to edit model server modal
Make data connection bucket field mandatory
Check serving runtime kind at creation time
Update ovms and add Caikit Custom Serving Runtime
Create dashboard configuration to control kserve and modelmesh
Adapt custom serving runtimes to KServe
Update ovms with gpu support and remove the other template
Add check for dsc status and utility types to check serving platform availablity
handle kserve in global model serving page
Refactor model serving section on the project details page
Add empty state when no serving platform is enabled
@openshift-merge-robot openshift-merge-robot added the needs-rebase PR needs to be rebased label Nov 10, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase PR needs to be rebased label Nov 10, 2023
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions I would like answered asap:

The rest is followup / cleanup. Log tickets as needed to work through things.

frontend/src/api/apiMergeUtils.ts Show resolved Hide resolved
frontend/src/api/k8s/inferenceServices.ts Show resolved Hide resolved
Comment on lines -111 to +116
export const addSupportModelMeshProject = (name: string): Promise<string> =>
axios(`/api/namespaces/${name}/1`).then((response) => {
export const addSupportServingPlatformProject = (
name: string,
servingPlatform: NamespaceApplicationCase,
): Promise<string> =>
axios(`/api/namespaces/${name}/${servingPlatform}`).then((response) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should have improved this not to use /1 if we were just going to place the constant values in the frontend code. We could have gone with a more reasonable endpoint name.

@lucferbux please log a tech debt for us to look into this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christianvogt would this not better serve to be part of the table folder of items? Do we expect this to not be table related? "Resource Actions" technically doesn't need to be, but I imagine this is the only use-case we know today for "marked for delete"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's table specific and probably should be put into the table dir.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code, please remove.

Comment on lines +85 to +97
existingTemplate.metadata.annotations
? {
op: 'replace',
path: '/metadata/annotations/opendatahub.io~1modelServingSupport',
value: JSON.stringify(platforms),
}
: {
op: 'add',
path: '/metadata/annotations',
value: {
'opendatahub.io/modelServingSupport': JSON.stringify(platforms),
},
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dupe code. frontend/src/api/k8s/templates.ts -- we should resolve this... it'll be a maintenance annoyance.

DaoDaoNoCode and others added 5 commits November 13, 2023 13:57
Change empty global model mesh serving link text to project display name
Change base image of text generation inference container
Show error state on global serving page when the platform in not installed
Copy link
Contributor

openshift-ci bot commented Nov 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 34c5a09 into main Nov 13, 2023
6 checks passed
@andrewballantyne andrewballantyne deleted the f/model-serving branch November 23, 2023 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tracker]: Model Serving v2 - KServe Support
8 participants