Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: query param might be empty string value #49

Merged
merged 1 commit into from
Mar 25, 2024
Merged

fix: query param might be empty string value #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/apiutils/api_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ func Of[E any](e E) *E {
return &e
}

func StrPtr(notEmpty string) *string {
if notEmpty == "" {
return nil
}
return &notEmpty
}

func BuildListOption(pageSize string, orderBy model.OrderByField, sortOrder model.SortOrder, nextPageToken string) (api.ListOptions, error) {
var pageSizeInt32 *int32
if pageSize != "" {
Expand Down
20 changes: 10 additions & 10 deletions internal/server/openapi/api_model_registry_service_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, &registeredmodelId)
result, err := s.coreApi.UpsertModelVersion(&modelVersion, apiutils.StrPtr(registeredmodelId))
if err != nil {
return Response(500, model.Error{Message: err.Error()}), nil
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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, &registeredModelId, &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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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, &registeredmodelId)
result, err := s.coreApi.GetModelVersions(listOpts, apiutils.StrPtr(registeredmodelId))
if err != nil {
return Response(500, model.Error{Message: err.Error()}), nil
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/core/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 38 additions & 0 deletions test/robot/MRkeywords.resource
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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"
Expand All @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions test/robot/UserStory.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading