Skip to content

Commit b10b1a7

Browse files
Merge pull request #1424 from kubeflow/main
[pull] main from kubeflow:main
2 parents 0de10f8 + ac1101a commit b10b1a7

27 files changed

Lines changed: 1356 additions & 197 deletions

api/openapi/catalog.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,10 +875,12 @@ components:
875875
description: |-
876876
Operational status of a catalog source.
877877
- `available`: The source is functioning correctly and models can be retrieved
878+
- `partially-available`: The source loaded some models successfully but encountered errors with others
878879
- `error`: The source is experiencing issues and cannot provide models
879880
- `disabled`: The source has been intentionally disabled
880881
enum:
881882
- available
883+
- partially-available
882884
- error
883885
- disabled
884886
type: string

api/openapi/src/catalog.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,10 +713,12 @@ components:
713713
description: |-
714714
Operational status of a catalog source.
715715
- `available`: The source is functioning correctly and models can be retrieved
716+
- `partially-available`: The source loaded some models successfully but encountered errors with others
716717
- `error`: The source is experiencing issues and cannot provide models
717718
- `disabled`: The source has been intentionally disabled
718719
enum:
719720
- available
721+
- partially-available
720722
- error
721723
- disabled
722724
type: string

catalog/clients/python/src/catalog_openapi/models/catalog_source_status.py

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

catalog/clients/python/tests/test_sources.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def test_source_status_values(self, api_client: CatalogAPIClient, suppress_ssl_w
145145
assert status is not None or enabled is not None
146146

147147
if status:
148-
valid_statuses = {"available", "disabled", "error", "loading", "pending"}
148+
valid_statuses = {"available", "partially-available", "disabled", "error", "loading", "pending"}
149149
assert status in valid_statuses, f"Unexpected status: {status}"
150150

151151
def test_enabled_and_status_consistency(self, api_client: CatalogAPIClient, suppress_ssl_warnings: None):

catalog/internal/catalog/catalog.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,13 @@ import (
99
)
1010

1111
type ListModelsParams struct {
12-
Query string
13-
FilterQuery string
14-
SourceIDs []string
15-
SourceLabels []string
16-
PageSize int32
17-
OrderBy model.OrderByField
18-
SortOrder model.SortOrder
19-
NextPageToken *string
20-
Recommended bool
21-
TargetRPS int32
22-
LatencyProperty string
23-
RPSProperty string
24-
HardwareCountProperty string
25-
HardwareTypeProperty string
12+
Query string
13+
FilterQuery string
14+
SourceIDs []string
15+
PageSize int32
16+
OrderBy model.OrderByField
17+
SortOrder model.SortOrder
18+
NextPageToken *string
2619
}
2720

2821
type ListArtifactsParams struct {
@@ -63,7 +56,8 @@ type APIProvider interface {
6356
// FindModelsWithRecommendedLatency returns models sorted by recommended latency using Pareto filtering.
6457
// Models without computable latency appear at the end of results.
6558
// If sourceIDs is provided, filter models by source IDs.
66-
FindModelsWithRecommendedLatency(ctx context.Context, pagination mrmodels.Pagination, paretoParams dbmodels.ParetoFilteringParams, sourceIDs []string) (*model.CatalogModelList, error)
59+
// If query is provided, filter models by text search.
60+
FindModelsWithRecommendedLatency(ctx context.Context, pagination mrmodels.Pagination, paretoParams dbmodels.ParetoFilteringParams, sourceIDs []string, query string) (*model.CatalogModelList, error)
6761

6862
// GetArtifacts returns all artifacts for a particular model. If no
6963
// model is found with that name, it returns nil. If the model is

catalog/internal/catalog/db_catalog.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,21 +85,6 @@ func (d *dbCatalogImpl) ListModels(ctx context.Context, params ListModelsParams)
8585
}
8686

8787
sourceIDs := params.SourceIDs
88-
if len(sourceIDs) == 0 && len(params.SourceLabels) > 0 {
89-
sources := d.sources.ByLabel(params.SourceLabels)
90-
if len(sources) == 0 {
91-
// No matching sources, so no matching models.
92-
return apimodels.CatalogModelList{
93-
Items: make([]apimodels.CatalogModel, 0),
94-
PageSize: pageSize,
95-
}, nil
96-
}
97-
98-
sourceIDs = make([]string, len(sources))
99-
for i, source := range sources {
100-
sourceIDs[i] = source.Id
101-
}
102-
}
10388

10489
modelsList, err := d.catalogModelRepository.List(dbmodels.CatalogModelListOptions{
10590
SourceIDs: &sourceIDs,
@@ -684,15 +669,22 @@ func (d *dbCatalogImpl) FindModelsWithRecommendedLatency(
684669
pagination mrmodels.Pagination,
685670
paretoParams dbmodels.ParetoFilteringParams,
686671
sourceIDs []string,
672+
query string,
687673
) (*apimodels.CatalogModelList, error) {
688674
// Get all models first (without pagination)
689675
var sourceIDsPtr *[]string
690676
if len(sourceIDs) > 0 {
691677
sourceIDsPtr = &sourceIDs
692678
}
693679

680+
var queryPtr *string
681+
if query != "" {
682+
queryPtr = &query
683+
}
684+
694685
allModels, err := d.catalogModelRepository.List(dbmodels.CatalogModelListOptions{
695686
SourceIDs: sourceIDsPtr,
687+
Query: queryPtr,
696688
Pagination: mrmodels.Pagination{
697689
FilterQuery: pagination.FilterQuery,
698690
PageSize: apiutils.Of(int32(0)), // Get all models

catalog/internal/catalog/db_catalog_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2091,6 +2091,7 @@ func TestFindModelsWithRecommendedLatency(t *testing.T) {
20912091
pagination,
20922092
paretoParams,
20932093
[]string{"latency-test-source"}, // Filter by this test's source ID
2094+
"", // No query filter
20942095
)
20952096

20962097
require.NoError(t, err)

catalog/internal/catalog/hf_catalog.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ func (p *hfModelProvider) getModelsFromHF(ctx context.Context) ([]ModelProviderR
549549
}
550550

551551
if len(failedModels) > 0 {
552-
return records, fmt.Errorf("Failed to fetch some models, ensure models exist and are accessible with given credentials. Failed models: %v", failedModels)
552+
return records, &PartiallyAvailableError{FailedModels: failedModels}
553553
}
554554

555555
return records, nil

catalog/internal/catalog/integration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func BenchmarkRecommendedLatencySorting(b *testing.B) {
150150
for i := 0; i < b.N; i++ {
151151
_, err := provider.FindModelsWithRecommendedLatency(ctx, mr_models.Pagination{
152152
PageSize: apiutils.Of(int32(20)),
153-
}, paretoParams, []string{"benchmark-source"})
153+
}, paretoParams, []string{"benchmark-source"}, "")
154154

155155
require.NoError(b, err)
156156
}

catalog/internal/catalog/loader.go

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,30 @@ import (
1919

2020
// Source status constants matching the OpenAPI enum values
2121
const (
22-
SourceStatusAvailable = "available"
23-
SourceStatusError = "error"
24-
SourceStatusDisabled = "disabled"
22+
SourceStatusAvailable = "available"
23+
SourceStatusPartiallyAvailable = "partially-available"
24+
SourceStatusError = "error"
25+
SourceStatusDisabled = "disabled"
2526
)
2627

28+
// PartiallyAvailableError indicates that a source loaded some models successfully
29+
// but encountered errors with others.
30+
type PartiallyAvailableError struct {
31+
FailedModels []string
32+
}
33+
34+
func (e *PartiallyAvailableError) Error() string {
35+
return fmt.Sprintf("Failed to fetch some models, ensure models exist and are accessible with given credentials. Failed models: %v", e.FailedModels)
36+
}
37+
38+
func (e *PartiallyAvailableError) Is(target error) bool {
39+
_, ok := target.(*PartiallyAvailableError)
40+
return ok
41+
}
42+
43+
// ErrPartiallyAvailable is used with errors.Is() to check for this error type.
44+
var ErrPartiallyAvailable error = &PartiallyAvailableError{}
45+
2746
// ModelProviderRecord contains one model and its associated artifacts.
2847
type ModelProviderRecord struct {
2948
Model dbmodels.CatalogModel
@@ -450,9 +469,9 @@ func (l *Loader) readProviderRecords(ctx context.Context) <-chan ModelProviderRe
450469
// Only save status if context is still valid (no reload in progress)
451470
if ctx.Err() == nil {
452471
// Check if there was a partial error (some models failed to load)
453-
if r.Error != nil {
454-
glog.Errorf("%s: partial error after loading models: %v", sourceID, r.Error)
455-
l.saveSourceStatus(sourceID, SourceStatusError, r.Error.Error())
472+
if errors.Is(r.Error, ErrPartiallyAvailable) {
473+
glog.Warningf("%s: partial error after loading models: %v", sourceID, r.Error)
474+
l.saveSourceStatus(sourceID, SourceStatusPartiallyAvailable, r.Error.Error())
456475
} else {
457476
l.saveSourceStatus(sourceID, SourceStatusAvailable, "")
458477
}
@@ -620,7 +639,7 @@ func (l *Loader) removeOrphanedModelsFromSource(sourceID string, valid mapset.Se
620639
func (l *Loader) saveSourceStatus(sourceID, status string, errorMsg string) {
621640
// Validate status is a valid enum value
622641
switch status {
623-
case SourceStatusAvailable, SourceStatusError, SourceStatusDisabled:
642+
case SourceStatusAvailable, SourceStatusPartiallyAvailable, SourceStatusError, SourceStatusDisabled:
624643
// valid
625644
default:
626645
glog.Errorf("invalid status %q for source %s", status, sourceID)

0 commit comments

Comments
 (0)