-
Notifications
You must be signed in to change notification settings - Fork 168
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
Update 'Resource name' fields to meet UX guidelines: Model registries and Inference services #3464
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d52e81c
to
cfbb904
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3464 +/- ##
==========================================
- Coverage 85.24% 85.23% -0.02%
==========================================
Files 1355 1354 -1
Lines 31074 31066 -8
Branches 8663 8662 -1
==========================================
- Hits 26489 26478 -11
- Misses 4585 4588 +3
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
I tested out the PR image and everything seems to work good for single and multi model serving. I didn't check model registry changes though |
cfbb904
to
b88cc2c
Compare
@jeff-phillips-18 the resource name isn't being used when deploying a model. It is always setting the resource name to the translated display name. |
b88cc2c
to
21354c3
Compare
… and Inference services
21354c3
to
bfb4ea5
Compare
); | ||
|
||
return sectionless ? contents : <FormSection style={{ margin: 0 }}>{contents}</FormSection>; |
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.
I think it makes sense to drop the section entirely and let the parent manage section considering we previously already removed the margin from the section.
Towards RHOAIENG-14746
Description
Updates create/edit modals to use
K8sNameDescriptionField
component in order to meet current UX guidelines.Modals updated:
Screen shots
Deploy model
Edit model
Model registry - Deploy model
How Has This Been Tested?
Navigate to the Model Serving section
Deploy model
Edit resource name
link.Navigate to the Model Serving section
Edit
Navigate to the Model Registry section
Deploy
Deploy
is enabledEdit resource name
link.Deploy
button is disabledDeploy
button is enabledEdit
Navigate to a project that is not configured for kserve or model mesh
Select single-model
from the Single-model serving platform cardDeploy model
Deploy
is enabledEdit resource name
link.Deploy
button is disabledDeploy
button is enabledEdit
Navigate to a project this model servers and select the
Models
tabAdd model server
Edit resource name
link.Navigate to a project this model servers and select the
Models
tabEdit
Test Impact
Updated unit tests for new fields
Request review criteria:
/cc @simrandhaliw