Skip to content

Commit 5f85686

Browse files
Al-PragliolaAl-Pragliola
authored
feat: catalog source status (kubeflow#1982)
* feat: catalog source status Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> * fix: update .gitattributes file Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> * fix: delete source entries on reload Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> * fix: bugs on the sources load Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> * feat: add more unit tests Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> * feat: apply suggestions from @pboyd Signed-off-by: Alessio Pragliola <seth.pro@gmail.com> --------- Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>
1 parent b367a2a commit 5f85686

21 files changed

+1215
-30
lines changed

‎.gitattributes‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ catalog/pkg/openapi/model_catalog_source.go linguist-generated=true
3030
catalog/pkg/openapi/model_catalog_source_list.go linguist-generated=true
3131
catalog/pkg/openapi/model_catalog_source_preview_response.go linguist-generated=true
3232
catalog/pkg/openapi/model_catalog_source_preview_response_all_of_summary.go linguist-generated=true
33+
catalog/pkg/openapi/model_catalog_source_status.go linguist-generated=true
3334
catalog/pkg/openapi/model_error.go linguist-generated=true
3435
catalog/pkg/openapi/model_field_filter.go linguist-generated=true
3536
catalog/pkg/openapi/model_filter_option.go linguist-generated=true

‎api/openapi/catalog.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,15 @@ components:
725725
type: array
726726
items:
727727
type: string
728+
status:
729+
$ref: "#/components/schemas/CatalogSourceStatus"
730+
description: Current operational status of the catalog source.
731+
error:
732+
description: |-
733+
Detailed error information when the status is "Error".
734+
This field is null or empty when the source is functioning normally.
735+
type: string
736+
nullable: true
728737
includedModels:
729738
description: |-
730739
Optional list of glob patterns for models to include. If specified, only models matching
@@ -818,6 +827,17 @@ components:
818827
- items
819828
- summary
820829
- $ref: "#/components/schemas/BaseResourceList"
830+
CatalogSourceStatus:
831+
description: |-
832+
Operational status of a catalog source.
833+
- `available`: The source is functioning correctly and models can be retrieved
834+
- `error`: The source is experiencing issues and cannot provide models
835+
- `disabled`: The source has been intentionally disabled
836+
enum:
837+
- available
838+
- error
839+
- disabled
840+
type: string
821841
Error:
822842
description: Error code and message.
823843
required:

‎api/openapi/src/catalog.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,15 @@ components:
610610
type: array
611611
items:
612612
type: string
613+
status:
614+
$ref: "#/components/schemas/CatalogSourceStatus"
615+
description: Current operational status of the catalog source.
616+
error:
617+
description: |-
618+
Detailed error information when the status is "Error".
619+
This field is null or empty when the source is functioning normally.
620+
type: string
621+
nullable: true
613622
includedModels:
614623
description: |-
615624
Optional list of glob patterns for models to include. If specified, only models matching
@@ -656,6 +665,17 @@ components:
656665
type: array
657666
items:
658667
type: string
668+
CatalogSourceStatus:
669+
description: |-
670+
Operational status of a catalog source.
671+
- `available`: The source is functioning correctly and models can be retrieved
672+
- `error`: The source is experiencing issues and cannot provide models
673+
- `disabled`: The source has been intentionally disabled
674+
enum:
675+
- available
676+
- error
677+
- disabled
678+
type: string
659679
CatalogSourceList:
660680
description: List of CatalogSource entities.
661681
allOf:

‎catalog/cmd/catalog.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func runCatalogServer(cmd *cobra.Command, args []string) error {
6363
getRepo[models.CatalogArtifactRepository](repoSet),
6464
getRepo[models.CatalogModelArtifactRepository](repoSet),
6565
getRepo[models.CatalogMetricsArtifactRepository](repoSet),
66+
getRepo[models.CatalogSourceRepository](repoSet),
6667
getRepo[models.PropertyOptionsRepository](repoSet),
6768
)
6869

@@ -85,7 +86,12 @@ func runCatalogServer(cmd *cobra.Command, args []string) error {
8586
return fmt.Errorf("error loading catalog sources: %v", err)
8687
}
8788

88-
svc := openapi.NewModelCatalogServiceAPIService(catalog.NewDBCatalog(services, loader.Sources), loader.Sources, loader.Labels)
89+
svc := openapi.NewModelCatalogServiceAPIService(
90+
catalog.NewDBCatalog(services, loader.Sources),
91+
loader.Sources,
92+
loader.Labels,
93+
services.CatalogSourceRepository,
94+
)
8995
ctrl := openapi.NewModelCatalogServiceAPIController(svc)
9096

9197
glog.Infof("Catalog API server listening on %s", catalogCfg.ListenAddress)

‎catalog/internal/catalog/catalog_test.go‎

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ func TestLoadCatalogSources(t *testing.T) {
2828
wantErr bool
2929
}{
3030
{
31-
name: "test-catalog-sources",
32-
args: args{catalogsPath: "testdata/test-catalog-sources.yaml"},
33-
want: []string{"catalog1", "catalog2"},
31+
name: "test-catalog-sources",
32+
args: args{catalogsPath: "testdata/test-catalog-sources.yaml"},
33+
want: []string{"catalog1", "catalog2"},
3434
},
3535
}
3636
for _, tt := range tests {
@@ -41,6 +41,7 @@ func TestLoadCatalogSources(t *testing.T) {
4141
&MockCatalogArtifactRepository{},
4242
&MockCatalogModelArtifactRepository{},
4343
&MockCatalogMetricsArtifactRepository{},
44+
&MockCatalogSourceRepository{},
4445
&MockPropertyOptionsRepository{},
4546
)
4647
loader := NewLoader(services, []string{tt.args.catalogsPath})
@@ -101,6 +102,7 @@ func TestLoadCatalogSourcesEnabledDisabled(t *testing.T) {
101102
&MockCatalogArtifactRepository{},
102103
&MockCatalogModelArtifactRepository{},
103104
&MockCatalogMetricsArtifactRepository{},
105+
&MockCatalogSourceRepository{},
104106
&MockPropertyOptionsRepository{},
105107
)
106108
loader := NewLoader(services, []string{tt.args.catalogsPath})
@@ -127,6 +129,7 @@ func TestLabelsValidation(t *testing.T) {
127129
&MockCatalogArtifactRepository{},
128130
&MockCatalogModelArtifactRepository{},
129131
&MockCatalogMetricsArtifactRepository{},
132+
&MockCatalogSourceRepository{},
130133
&MockPropertyOptionsRepository{},
131134
)
132135

@@ -257,6 +260,7 @@ func TestCatalogSourceLabelsDefaultToEmptySlice(t *testing.T) {
257260
&MockCatalogArtifactRepository{},
258261
&MockCatalogModelArtifactRepository{},
259262
&MockCatalogMetricsArtifactRepository{},
263+
&MockCatalogSourceRepository{},
260264
&MockPropertyOptionsRepository{},
261265
)
262266
loader := NewLoader(services, []string{tt.args.catalogsPath})
@@ -295,6 +299,7 @@ func TestLoadCatalogSourcesWithMockRepositories(t *testing.T) {
295299
mockArtifactRepo,
296300
mockModelArtifactRepo,
297301
mockMetricsArtifactRepo,
302+
&MockCatalogSourceRepository{},
298303
&MockPropertyOptionsRepository{},
299304
)
300305

@@ -424,6 +429,7 @@ func TestLoadCatalogSourcesWithRepositoryErrors(t *testing.T) {
424429
mockArtifactRepo,
425430
mockModelArtifactRepo,
426431
mockMetricsArtifactRepo,
432+
&MockCatalogSourceRepository{},
427433
&MockPropertyOptionsRepository{},
428434
)
429435

@@ -501,6 +507,7 @@ func TestLoadCatalogSourcesWithNilEnabled(t *testing.T) {
501507
mockArtifactRepo,
502508
mockModelArtifactRepo,
503509
mockMetricsArtifactRepo,
510+
&MockCatalogSourceRepository{},
504511
&MockPropertyOptionsRepository{},
505512
)
506513

@@ -944,6 +951,58 @@ func (m *MockPropertyOptionsRepository) SetMockOptions(t dbmodels.PropertyOption
944951
m.MockOptions[t][typeID] = options
945952
}
946953

954+
// MockCatalogSourceRepository mocks the CatalogSourceRepository interface.
955+
type MockCatalogSourceRepository struct {
956+
Sources []dbmodels.CatalogSource
957+
}
958+
959+
func (m *MockCatalogSourceRepository) GetBySourceID(sourceID string) (dbmodels.CatalogSource, error) {
960+
for _, s := range m.Sources {
961+
if attrs := s.GetAttributes(); attrs != nil && attrs.Name != nil && *attrs.Name == sourceID {
962+
return s, nil
963+
}
964+
}
965+
return nil, nil
966+
}
967+
968+
func (m *MockCatalogSourceRepository) Save(source dbmodels.CatalogSource) (dbmodels.CatalogSource, error) {
969+
m.Sources = append(m.Sources, source)
970+
return source, nil
971+
}
972+
973+
func (m *MockCatalogSourceRepository) Delete(sourceID string) error {
974+
return nil
975+
}
976+
977+
func (m *MockCatalogSourceRepository) GetAll() ([]dbmodels.CatalogSource, error) {
978+
return m.Sources, nil
979+
}
980+
981+
func (m *MockCatalogSourceRepository) GetAllStatuses() (map[string]dbmodels.SourceStatus, error) {
982+
result := make(map[string]dbmodels.SourceStatus)
983+
for _, source := range m.Sources {
984+
if attrs := source.GetAttributes(); attrs != nil && attrs.Name != nil {
985+
status := dbmodels.SourceStatus{}
986+
if props := source.GetProperties(); props != nil {
987+
for _, prop := range *props {
988+
switch prop.Name {
989+
case "status":
990+
if prop.StringValue != nil {
991+
status.Status = *prop.StringValue
992+
}
993+
case "error":
994+
if prop.StringValue != nil {
995+
status.Error = *prop.StringValue
996+
}
997+
}
998+
}
999+
}
1000+
result[*attrs.Name] = status
1001+
}
1002+
}
1003+
return result, nil
1004+
}
1005+
9471006
func TestAPIProviderGetPerformanceArtifacts(t *testing.T) {
9481007
// This test verifies that the APIProvider interface has GetPerformanceArtifacts method
9491008
// The actual implementation is tested in db_catalog_test.go
@@ -954,6 +1013,7 @@ func TestAPIProviderGetPerformanceArtifacts(t *testing.T) {
9541013
&MockCatalogArtifactRepository{},
9551014
&MockCatalogModelArtifactRepository{},
9561015
&MockCatalogMetricsArtifactRepository{},
1016+
&MockCatalogSourceRepository{},
9571017
&MockPropertyOptionsRepository{},
9581018
)
9591019
provider := NewDBCatalog(services, nil)
@@ -981,6 +1041,7 @@ func TestAPIProviderInterface(t *testing.T) {
9811041
&MockCatalogArtifactRepository{},
9821042
&MockCatalogModelArtifactRepository{},
9831043
&MockCatalogMetricsArtifactRepository{},
1044+
&MockCatalogSourceRepository{},
9841045
&MockPropertyOptionsRepository{},
9851046
)
9861047
var provider APIProvider = NewDBCatalog(services, nil)

‎catalog/internal/catalog/db_catalog_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func TestDBCatalog(t *testing.T) {
3333
catalogModelTypeID := getCatalogModelTypeIDForDBTest(t, sharedDB)
3434
modelArtifactTypeID := getCatalogModelArtifactTypeIDForDBTest(t, sharedDB)
3535
metricsArtifactTypeID := getCatalogMetricsArtifactTypeIDForDBTest(t, sharedDB)
36+
catalogSourceTypeID := getCatalogSourceTypeIDForDBTest(t, sharedDB)
3637

3738
// Create repositories
3839
catalogModelRepo := service.NewCatalogModelRepository(sharedDB, catalogModelTypeID)
@@ -42,12 +43,14 @@ func TestDBCatalog(t *testing.T) {
4243
})
4344
modelArtifactRepo := service.NewCatalogModelArtifactRepository(sharedDB, modelArtifactTypeID)
4445
metricsArtifactRepo := service.NewCatalogMetricsArtifactRepository(sharedDB, metricsArtifactTypeID)
46+
catalogSourceRepo := service.NewCatalogSourceRepository(sharedDB, catalogSourceTypeID)
4547

4648
svcs := service.NewServices(
4749
catalogModelRepo,
4850
catalogArtifactRepo,
4951
modelArtifactRepo,
5052
metricsArtifactRepo,
53+
catalogSourceRepo,
5154
service.NewPropertyOptionsRepository(sharedDB),
5255
)
5356

@@ -1415,6 +1418,7 @@ func TestDBCatalog_GetPerformanceArtifactsWithService(t *testing.T) {
14151418
catalogModelTypeID := getCatalogModelTypeIDForDBTest(t, sharedDB)
14161419
modelArtifactTypeID := getCatalogModelArtifactTypeIDForDBTest(t, sharedDB)
14171420
metricsArtifactTypeID := getCatalogMetricsArtifactTypeIDForDBTest(t, sharedDB)
1421+
catalogSourceTypeID := getCatalogSourceTypeIDForDBTest(t, sharedDB)
14181422

14191423
// Create repositories
14201424
catalogModelRepo := service.NewCatalogModelRepository(sharedDB, catalogModelTypeID)
@@ -1424,12 +1428,14 @@ func TestDBCatalog_GetPerformanceArtifactsWithService(t *testing.T) {
14241428
})
14251429
modelArtifactRepo := service.NewCatalogModelArtifactRepository(sharedDB, modelArtifactTypeID)
14261430
metricsArtifactRepo := service.NewCatalogMetricsArtifactRepository(sharedDB, metricsArtifactTypeID)
1431+
catalogSourceRepo := service.NewCatalogSourceRepository(sharedDB, catalogSourceTypeID)
14271432

14281433
services := service.NewServices(
14291434
catalogModelRepo,
14301435
catalogArtifactRepo,
14311436
modelArtifactRepo,
14321437
metricsArtifactRepo,
1438+
catalogSourceRepo,
14331439
service.NewPropertyOptionsRepository(sharedDB),
14341440
)
14351441

@@ -1571,3 +1577,12 @@ func getCatalogMetricsArtifactTypeIDForDBTest(t *testing.T, db *gorm.DB) int32 {
15711577
}
15721578
return typeRecord.ID
15731579
}
1580+
1581+
func getCatalogSourceTypeIDForDBTest(t *testing.T, db *gorm.DB) int32 {
1582+
var typeRecord schema.Type
1583+
err := db.Where("name = ?", service.CatalogSourceTypeName).First(&typeRecord).Error
1584+
if err != nil {
1585+
require.NoError(t, err, "Failed to query CatalogSource type")
1586+
}
1587+
return typeRecord.ID
1588+
}

0 commit comments

Comments
 (0)