-
Notifications
You must be signed in to change notification settings - Fork 167
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
Add info alert to kserve on settings page #2082
Add info alert to kserve on settings page #2082
Conversation
@kaedward @vconzola Could you check the text in the alert here? The mockup only shows the alert text when we uncheck a multi-model serving platform, however, we want to apply the same behavior to the single model serving platform. Should we use a more general text as is shown in the description for both settings or do you have any better ideas to show a different alert text for the single model serving platform? |
/lgtm Let's wait for UX first |
@lucferbux @DaoDaoNoCode Is the scenario here that both boxes were checked and the user just unchecked multi-model serving? If so, I'd say some thing like, "Disabling multi-model serving means that models in new projects or existing projects with no currently deployed models will be deployed from their own model server. Existing projects with currently deployed models will continue to use the serving platform selected for that project." @kaedward - Please review. |
@vconzola It's the check for either checkbox. It also could be the user selected the single model serving platform before but unchecked it later, we also need to show the info alert, so we should either show a generic info alert for both situations or create different alert text for these 2 situations. |
@DaoDaoNoCode I think it would be better to have different alert text for each scenario. For the single model serving case I'd suggest something like, ""Disabling single model serving means that models in new projects or existing projects with no currently deployed models will be deployed from a shared model server. Existing projects with currently deployed models will continue to use the serving platform selected for that project." The italics just indicate changes from the multi-model use case text. |
@vconzola that sounds great! |
@vconzola Thanks for the suggestion! Could you check it again? |
9f31c3d
to
a9a0bfa
Compare
LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lucferbux 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 |
4ccc506
into
opendatahub-io:f/model-serving
Closes #2047
Description
Previously, we only showed this alert when
Multi-model serving platform
was checked but unchecked later. In this PR, we apply the same behavior toSingle model serving platform
as well. No matter which checkbox was checked before but unchecked later, we show this info alert with the following text.How Has This Been Tested?
Test Impact
Add an integration test check to see if this alert also shows when single model serving platform was checked but unchecked later.
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main