From 2bdd8b6f5b4236024afcaaad27b901d3b9c14d44 Mon Sep 17 00:00:00 2001 From: Matteo Mortari Date: Thu, 21 Mar 2024 12:24:51 +0100 Subject: [PATCH] fix: query param might be empty string value ...handle gin-generated code by openapi-codegen internally, so to make propert string ptr semantic as expected by core lib, as a query param might not even be passed during requests Signed-off-by: Matteo Mortari --- internal/apiutils/api_utils.go | 7 ++++ .../api_model_registry_service_service.go | 20 +++++----- pkg/core/core.go | 2 + test/robot/MRkeywords.resource | 38 +++++++++++++++++++ test/robot/UserStory.robot | 6 +++ 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/internal/apiutils/api_utils.go b/internal/apiutils/api_utils.go index 6c5a10da..d601097c 100644 --- a/internal/apiutils/api_utils.go +++ b/internal/apiutils/api_utils.go @@ -51,6 +51,13 @@ func Of[E any](e E) *E { return &e } +func StrPtr(notEmpty string) *string { + if notEmpty == "" { + return nil + } + return ¬Empty +} + func BuildListOption(pageSize string, orderBy model.OrderByField, sortOrder model.SortOrder, nextPageToken string) (api.ListOptions, error) { var pageSizeInt32 *int32 if pageSize != "" { diff --git a/internal/server/openapi/api_model_registry_service_service.go b/internal/server/openapi/api_model_registry_service_service.go index 18ce3323..e5d5b98f 100644 --- a/internal/server/openapi/api_model_registry_service_service.go +++ b/internal/server/openapi/api_model_registry_service_service.go @@ -138,7 +138,7 @@ func (s *ModelRegistryServiceAPIService) CreateRegisteredModel(ctx context.Conte // CreateRegisteredModelVersion - Create a ModelVersion in RegisteredModel func (s *ModelRegistryServiceAPIService) CreateRegisteredModelVersion(ctx context.Context, registeredmodelId string, modelVersion model.ModelVersion) (ImplResponse, error) { - result, err := s.coreApi.UpsertModelVersion(&modelVersion, ®isteredmodelId) + result, err := s.coreApi.UpsertModelVersion(&modelVersion, apiutils.StrPtr(registeredmodelId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -166,7 +166,7 @@ func (s *ModelRegistryServiceAPIService) CreateServingEnvironment(ctx context.Co // FindInferenceService - Get an InferenceServices that matches search parameters. func (s *ModelRegistryServiceAPIService) FindInferenceService(ctx context.Context, name string, externalId string, parentResourceId string) (ImplResponse, error) { - result, err := s.coreApi.GetInferenceServiceByParams(&name, &parentResourceId, &externalId) + result, err := s.coreApi.GetInferenceServiceByParams(apiutils.StrPtr(name), apiutils.StrPtr(parentResourceId), apiutils.StrPtr(externalId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -178,7 +178,7 @@ func (s *ModelRegistryServiceAPIService) FindInferenceService(ctx context.Contex // FindModelArtifact - Get a ModelArtifact that matches search parameters. func (s *ModelRegistryServiceAPIService) FindModelArtifact(ctx context.Context, name string, externalId string, parentResourceId string) (ImplResponse, error) { - result, err := s.coreApi.GetModelArtifactByParams(&name, &parentResourceId, &externalId) + result, err := s.coreApi.GetModelArtifactByParams(apiutils.StrPtr(name), apiutils.StrPtr(parentResourceId), apiutils.StrPtr(externalId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -190,7 +190,7 @@ func (s *ModelRegistryServiceAPIService) FindModelArtifact(ctx context.Context, // FindModelVersion - Get a ModelVersion that matches search parameters. func (s *ModelRegistryServiceAPIService) FindModelVersion(ctx context.Context, name string, externalId string, registeredModelId string) (ImplResponse, error) { - result, err := s.coreApi.GetModelVersionByParams(&name, ®isteredModelId, &externalId) + result, err := s.coreApi.GetModelVersionByParams(apiutils.StrPtr(name), apiutils.StrPtr(registeredModelId), apiutils.StrPtr(externalId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -202,7 +202,7 @@ func (s *ModelRegistryServiceAPIService) FindModelVersion(ctx context.Context, n // FindRegisteredModel - Get a RegisteredModel that matches search parameters. func (s *ModelRegistryServiceAPIService) FindRegisteredModel(ctx context.Context, name string, externalID string) (ImplResponse, error) { - result, err := s.coreApi.GetRegisteredModelByParams(&name, &externalID) + result, err := s.coreApi.GetRegisteredModelByParams(apiutils.StrPtr(name), apiutils.StrPtr(externalID)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -214,7 +214,7 @@ func (s *ModelRegistryServiceAPIService) FindRegisteredModel(ctx context.Context // FindServingEnvironment - Find ServingEnvironment func (s *ModelRegistryServiceAPIService) FindServingEnvironment(ctx context.Context, name string, externalID string) (ImplResponse, error) { - result, err := s.coreApi.GetServingEnvironmentByParams(&name, &externalID) + result, err := s.coreApi.GetServingEnvironmentByParams(apiutils.StrPtr(name), apiutils.StrPtr(externalID)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -229,7 +229,7 @@ func (s *ModelRegistryServiceAPIService) GetEnvironmentInferenceServices(ctx con if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } - result, err := s.coreApi.GetInferenceServices(listOpts, &servingenvironmentId, nil) + result, err := s.coreApi.GetInferenceServices(listOpts, apiutils.StrPtr(servingenvironmentId), nil) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -266,7 +266,7 @@ func (s *ModelRegistryServiceAPIService) GetInferenceServiceServes(ctx context.C if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } - result, err := s.coreApi.GetServeModels(listOpts, &inferenceserviceId) + result, err := s.coreApi.GetServeModels(listOpts, apiutils.StrPtr(inferenceserviceId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -347,7 +347,7 @@ func (s *ModelRegistryServiceAPIService) GetModelVersionArtifacts(ctx context.Co if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } - result, err := s.coreApi.GetArtifacts(listOpts, &modelversionId) + result, err := s.coreApi.GetArtifacts(listOpts, apiutils.StrPtr(modelversionId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } @@ -391,7 +391,7 @@ func (s *ModelRegistryServiceAPIService) GetRegisteredModelVersions(ctx context. if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } - result, err := s.coreApi.GetModelVersions(listOpts, ®isteredmodelId) + result, err := s.coreApi.GetModelVersions(listOpts, apiutils.StrPtr(registeredmodelId)) if err != nil { return Response(500, model.Error{Message: err.Error()}), nil } diff --git a/pkg/core/core.go b/pkg/core/core.go index dc0ec9fe..afb0eaf8 100644 --- a/pkg/core/core.go +++ b/pkg/core/core.go @@ -245,6 +245,7 @@ func (serv *ModelRegistryService) GetRegisteredModelByParams(name *string, exter } else { return nil, fmt.Errorf("invalid parameters call, supply either name or externalId") } + glog.Info("filterQuery ", filterQuery) getByParamsResp, err := serv.mlmdClient.GetContextsByType(context.Background(), &proto.GetContextsByTypeRequest{ TypeName: &serv.nameConfig.RegisteredModelTypeName, @@ -779,6 +780,7 @@ func (serv *ModelRegistryService) GetModelArtifactByParams(artifactName *string, } else { return nil, fmt.Errorf("invalid parameters call, supply either (artifactName and modelVersionId), or externalId") } + glog.Info("filterQuery ", filterQuery) artifactsResponse, err := serv.mlmdClient.GetArtifactsByType(context.Background(), &proto.GetArtifactsByTypeRequest{ TypeName: &serv.nameConfig.ModelArtifactTypeName, diff --git a/test/robot/MRkeywords.resource b/test/robot/MRkeywords.resource index 2bd25b44..46bc0f05 100644 --- a/test/robot/MRkeywords.resource +++ b/test/robot/MRkeywords.resource @@ -121,6 +121,18 @@ I get RegisteredModelByID END RETURN ${result} +I findRegisteredModel by name + [Arguments] ${name} + IF $MODE == "REST" + ${resp}= GET url=http://${MR_HOST}:${MR_PORT}/api/model_registry/v1alpha3/registered_model?name=${name} expected_status=200 + ${result} Set Variable ${resp.json()} + Log to console ${resp.json()} + ELSE + Log to console ${MODE} + Fail Not Implemented + END + RETURN ${result} + I get ModelVersionByID [Arguments] ${id} @@ -135,6 +147,19 @@ I get ModelVersionByID RETURN ${result} +I findModelVersion by name and parentResourceId + [Arguments] ${name} ${parentResourceId} + IF $MODE == "REST" + ${resp}= GET url=http://${MR_HOST}:${MR_PORT}/api/model_registry/v1alpha3/model_version?name=${name}&parentResourceId=${parentResourceId} expected_status=200 + ${result} Set Variable ${resp.json()} + Log to console ${resp.json()} + ELSE + Log to console ${MODE} + Fail Not Implemented + END + RETURN ${result} + + I get ModelArtifactByID [Arguments] ${id} IF $MODE == "REST" @@ -148,6 +173,19 @@ I get ModelArtifactByID RETURN ${result} +I findModelArtifact by name and parentResourceId + [Arguments] ${name} ${parentResourceId} + IF $MODE == "REST" + ${resp}= GET url=http://${MR_HOST}:${MR_PORT}/api/model_registry/v1alpha3/model_artifact?name=${name}&parentResourceId=${parentResourceId} expected_status=200 + ${result} Set Variable ${resp.json()} + Log to console ${resp.json()} + ELSE + Log to console ${MODE} + Fail Not Implemented + END + RETURN ${result} + + I get ArtifactsByModelVersionID [Arguments] ${id} IF $MODE == "REST" diff --git a/test/robot/UserStory.robot b/test/robot/UserStory.robot index 4fa234c5..6fe598d1 100644 --- a/test/robot/UserStory.robot +++ b/test/robot/UserStory.robot @@ -29,10 +29,16 @@ As a MLOps engineer I would like to store a description of the model ${aId} And I create a child ModelArtifact modelversionId=${vId} payload=&{model_artifact} ${r} Then I get RegisteredModelByID id=${rId} And Should be equal ${r["description"]} Lorem ipsum dolor sit amet + ${r} Then I findRegisteredModel by name name=${name} + And Should be equal ${r["description"]} Lorem ipsum dolor sit amet ${r} Then I get ModelVersionByID id=${vId} And Should be equal ${r["description"]} consectetur adipiscing elit + ${r} Then I findModelVersion by name and parentResourceId name=v1.2.3 parentResourceId=${rId} + And Should be equal ${r["description"]} consectetur adipiscing elit ${r} Then I get ModelArtifactByID id=${aId} And Should be equal ${r["description"]} sed do eiusmod tempor incididunt + ${r} Then I findModelArtifact by name and parentResourceId name=ModelArtifactName parentResourceId=${vId} + And Should be equal ${r["description"]} sed do eiusmod tempor incididunt As a MLOps engineer I would like to store a longer documentation for the model Set To Dictionary ${registered_model} description=Lorem ipsum dolor sit amet name=${name}