Model Catalog Settings Table#1899
Conversation
Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>
Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>
mturley
left a comment
mturley
left a comment
There was a problem hiding this comment.
Hmm - can you also change the BFF mock data so the mock default source with id "catalog-1" does not have excludedModels and includedModels? That would be nice for correctly showing the "Unfiltered" label.
Also, can you have the table default to no sort state applied, and pre-sort the data so that default sources appear at the top? This should be before we pass the data into the table, so if a user applies a sort it will override this.
Ugh, this doesn't actually appear to be possible the way the mod-arch-shared Table is implemented. Whatever, it's fine 😅
| const handleEnableToggle = (checked: boolean) => { | ||
| // For now, just show a notification as per requirements | ||
| // TODO: RHOAIENG-38347 - Implement actual enable/disable functionality | ||
| notification.info( | ||
| `Toggle ${checked ? 'enabled' : 'disabled'}`, | ||
| `"${catalogSourceConfig.name}" will be ${checked ? 'enabled' : 'disabled'} when functionality is implemented.`, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Heh I just meant a window.alert() native stub, but this notification system is so easy that this is fine 😅 I don't think an alert on enable/disable is part of the design but perhaps it should be, maybe we preserve this later
|
|
||
| const handleEnableToggle = (checked: boolean) => { | ||
| // For now, just show a notification as per requirements | ||
| // TODO: RHOAIENG-38347 - Implement actual enable/disable functionality |
There was a problem hiding this comment.
We should not reference RH Jira tickets upstream. Can you remove these RHOAIENG comments and just leave generic TODOs?
| sortable: false, | ||
| info: { | ||
| popover: | ||
| 'Shows whether all models from a source appear in the model catalog or if visibility is filtered.\n\nAll models: Every model from the source appears in the catalog.\n\nFiltered: Only specific models appear, based on the visibility settings for that source.', |
There was a problem hiding this comment.
These \n newline characters don't get rendered by the popover component, and the design includes a bulleted list which is not rendered here. This popover property is a ReactNode, you can pass JSX here instead and use PF components to lay out the popover contents.
| export enum CatalogSourceType { | ||
| YAML = 'yaml', | ||
| HUGGING_FACE = 'huggingface', | ||
| } |
There was a problem hiding this comment.
Can you move CatalogSourceType to clients/ui/frontend/src/app/modelCatalogTypes.ts and use it to replace the hard-coded type: 'yaml'; and type: 'huggingface'; in the source config types there?
Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>
| }; | ||
|
|
||
| const handleDeleteSource = () => { | ||
| // TODO: RHOAIENG-38351 - Implement actual delete functionality |
There was a problem hiding this comment.
There's still a RHOAIENG comment here
| /> | ||
| )} | ||
| </Td> | ||
| <Td dataLabel="Validation status">{/* TODO: RHOAIENG-38346 - Status implementation */}</Td> |
Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mturley The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
Description
Implemented the catalog source configurations table for the Model Catalog Settings page. This table displays a list of
CatalogSourceConfigobjects with the following features:How Has This Been Tested?
Screen.Recording.2025-11-20.at.3.11.29.PM.mov
Merge criteria:
All the commits have been signed-off (To pass the
DCOcheck)The commits have meaningful messages
Automated tests are provided as part of the PR for major new functionalities; testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work.
Code changes follow the kubeflow contribution guidelines.
For first time contributors: Please reach out to the Reviewers to ensure all tests are being run, ensuring the label
ok-to-testhas been added to the PR.If you have UI changes