Skip to content

Commit

Permalink
Fix GetModelArtifactByParams when no results (kubeflow#165)
Browse files Browse the repository at this point in the history
  • Loading branch information
lampajr authored Nov 19, 2023
1 parent 507e93f commit 9feaaf9
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 3 deletions.
12 changes: 9 additions & 3 deletions pkg/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,9 +699,15 @@ func (serv *modelRegistryService) GetModelArtifactByParams(artifactName *string,
if err != nil {
return nil, err
}

if len(artifactsResponse.Artifacts) > 1 {
return nil, fmt.Errorf("more than an artifact detected matching criteria: %v", artifactsResponse.Artifacts)
return nil, fmt.Errorf("multiple model artifacts found for artifactName=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(artifactName), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
}

if len(artifactsResponse.Artifacts) == 0 {
return nil, fmt.Errorf("no model artifacts found for artifactName=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(artifactName), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
}

artifact0 = artifactsResponse.Artifacts[0]

result, err := serv.mapper.MapToModelArtifact(artifact0)
Expand Down Expand Up @@ -1072,11 +1078,11 @@ func (serv *modelRegistryService) GetInferenceServiceByParams(name *string, pare
}

if len(getByParamsResp.Contexts) > 1 {
return nil, fmt.Errorf("multiple inference services found for versionName=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(name), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
return nil, fmt.Errorf("multiple inference services found for name=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(name), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
}

if len(getByParamsResp.Contexts) == 0 {
return nil, fmt.Errorf("no inference services found for versionName=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(name), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
return nil, fmt.Errorf("no inference services found for name=%v, parentResourceId=%v, externalId=%v", apiutils.ZeroIfNil(name), apiutils.ZeroIfNil(parentResourceId), apiutils.ZeroIfNil(externalId))
}

toReturn, err := serv.mapper.MapToInferenceService(getByParamsResp.Contexts[0])
Expand Down
66 changes: 66 additions & 0 deletions pkg/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,18 @@ func TestGetRegisteredModelById(t *testing.T) {
assertion.Equal(*registeredModel.CustomProperties, *getModelById.CustomProperties, "saved model custom props should match the original one")
}

func TestGetRegisteredModelByParamsWithNoResults(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)

// create mode registry service
service := initModelRegistryService(assertion, conn)

_, err := service.GetRegisteredModelByParams(of("not-present"), nil)
assertion.NotNil(err)
assertion.Equal("no registered models found for name=not-present, externalId=", err.Error())
}

func TestGetRegisteredModelByParamsName(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)
Expand Down Expand Up @@ -1198,6 +1210,20 @@ func TestGetModelVersionById(t *testing.T) {
assertion.Equal(*(*getById.CustomProperties)["author"].MetadataStringValue.StringValue, author, "saved author custom property should match the provided one")
}

func TestGetModelVersionByParamsWithNoResults(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)

// create mode registry service
service := initModelRegistryService(assertion, conn)

registeredModelId := registerModel(assertion, service, nil, nil)

_, err := service.GetModelVersionByParams(of("not-present"), &registeredModelId, nil)
assertion.NotNil(err)
assertion.Equal("no model versions found for versionName=not-present, parentResourceId=1, externalId=", err.Error())
}

func TestGetModelVersionByParamsName(t *testing.T) {
assertion, conn, client, teardown := setup(t)
defer teardown(t)
Expand Down Expand Up @@ -1726,6 +1752,20 @@ func TestGetModelArtifactByEmptyParams(t *testing.T) {
assertion.Equal("invalid parameters call, supply either (artifactName and parentResourceId), or externalId", err.Error())
}

func TestGetModelArtifactByParamsWithNoResults(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)

// create mode registry service
service := initModelRegistryService(assertion, conn)

modelVersionId := registerModelVersion(assertion, service, nil, nil, nil, nil)

_, err := service.GetModelArtifactByParams(of("not-present"), &modelVersionId, nil)
assertion.NotNil(err)
assertion.Equal("no model artifacts found for artifactName=not-present, parentResourceId=2, externalId=", err.Error())
}

func TestGetModelArtifacts(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)
Expand Down Expand Up @@ -1989,6 +2029,18 @@ func TestGetServingEnvironmentById(t *testing.T) {
assertion.Equal(*eut.CustomProperties, *getEntityById.CustomProperties, "saved custom props should match the original one")
}

func TestGetServingEnvironmentByParamsWithNoResults(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)

// create mode registry service
service := initModelRegistryService(assertion, conn)

_, err := service.GetServingEnvironmentByParams(of("not-present"), nil)
assertion.NotNil(err)
assertion.Equal("no serving environments found for name=not-present, externalId=", err.Error())
}

func TestGetServingEnvironmentByParamsName(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)
Expand Down Expand Up @@ -2595,6 +2647,20 @@ func TestGetModelVersionByInferenceServiceId(t *testing.T) {
assertion.Equal(createdVersion1Id, *getVModel.Id, "returned id shall be the specified one")
}

func TestGetInferenceServiceByParamsWithNoResults(t *testing.T) {
assertion, conn, _, teardown := setup(t)
defer teardown(t)

// create mode registry service
service := initModelRegistryService(assertion, conn)

parentResourceId := registerServingEnvironment(assertion, service, nil, nil)

_, err := service.GetInferenceServiceByParams(of("not-present"), &parentResourceId, nil)
assertion.NotNil(err)
assertion.Equal("no inference services found for name=not-present, parentResourceId=1, externalId=", err.Error())
}

func TestGetInferenceServiceByParamsName(t *testing.T) {
assertion, conn, client, teardown := setup(t)
defer teardown(t)
Expand Down

0 comments on commit 9feaaf9

Please sign in to comment.