diff --git a/api/openapi/model-registry.yaml b/api/openapi/model-registry.yaml index 362b9f6f..15e8cef7 100644 --- a/api/openapi/model-registry.yaml +++ b/api/openapi/model-registry.yaml @@ -1023,10 +1023,18 @@ components: - $ref: "#/components/schemas/BaseResourceList" RegisteredModelCreate: description: A registered model in model registry. A registered model has ModelVersion children. + required: + - name allOf: - type: object - $ref: "#/components/schemas/BaseResourceCreate" - $ref: "#/components/schemas/RegisteredModelUpdate" + properties: + name: + description: |- + The client provided name of the model. It must be unique among all the RegisteredModels of the same + type within a Model Registry instance and cannot be changed once set. + type: string RegisteredModelUpdate: description: A registered model in model registry. A registered model has ModelVersion children. allOf: @@ -1045,6 +1053,7 @@ components: ModelVersionCreate: description: Represents a ModelVersion belonging to a RegisteredModel. required: + - name - registeredModelId allOf: - $ref: "#/components/schemas/BaseResourceCreate" @@ -1054,6 +1063,11 @@ components: registeredModelId: description: ID of the `RegisteredModel` to which this version belongs. type: string + name: + description: |- + The client provided name of the model's version. It must be unique among all the ModelVersions of the same + type within a Model Registry instance and cannot be changed once set. + type: string ModelVersionUpdate: description: Represents a ModelVersion belonging to a RegisteredModel. allOf: diff --git a/internal/converter/generated/mlmd_openapi_converter.gen.go b/internal/converter/generated/mlmd_openapi_converter.gen.go index 6af53b5e..e140392c 100644 --- a/internal/converter/generated/mlmd_openapi_converter.gen.go +++ b/internal/converter/generated/mlmd_openapi_converter.gen.go @@ -122,7 +122,7 @@ func (c *MLMDToOpenAPIConverterImpl) ConvertModelVersion(source *proto.Context) xstring := *(*source).ExternalId openapiModelVersion.ExternalId = &xstring } - openapiModelVersion.Name = converter.MapNameFromOwned((*source).Name) + openapiModelVersion.Name = converter.MapName((*source).Name) openapiModelVersion.State = converter.MapModelVersionState((*source).Properties) openapiModelVersion.Author = converter.MapPropertyAuthor((*source).Properties) xstring2, err := converter.MapRegisteredModelIdFromOwned((*source).Name) @@ -152,8 +152,7 @@ func (c *MLMDToOpenAPIConverterImpl) ConvertRegisteredModel(source *proto.Contex openapiRegisteredModel.ExternalId = &xstring } if (*source).Name != nil { - xstring2 := *(*source).Name - openapiRegisteredModel.Name = &xstring2 + openapiRegisteredModel.Name = *(*source).Name } openapiRegisteredModel.Id = converter.Int64ToString((*source).Id) openapiRegisteredModel.CreateTimeSinceEpoch = converter.Int64ToString((*source).CreateTimeSinceEpoch) diff --git a/internal/converter/generated/openapi_converter.gen.go b/internal/converter/generated/openapi_converter.gen.go index 68dfc926..7d1c8c14 100644 --- a/internal/converter/generated/openapi_converter.gen.go +++ b/internal/converter/generated/openapi_converter.gen.go @@ -239,10 +239,7 @@ func (c *OpenAPIConverterImpl) ConvertModelVersionCreate(source *openapi.ModelVe xstring2 := *(*source).ExternalId openapiModelVersion.ExternalId = &xstring2 } - if (*source).Name != nil { - xstring3 := *(*source).Name - openapiModelVersion.Name = &xstring3 - } + openapiModelVersion.Name = (*source).Name if (*source).State != nil { openapiModelVersionState, err := c.openapiModelVersionStateToOpenapiModelVersionState(*(*source).State) if err != nil { @@ -251,8 +248,8 @@ func (c *OpenAPIConverterImpl) ConvertModelVersionCreate(source *openapi.ModelVe openapiModelVersion.State = &openapiModelVersionState } if (*source).Author != nil { - xstring4 := *(*source).Author - openapiModelVersion.Author = &xstring4 + xstring3 := *(*source).Author + openapiModelVersion.Author = &xstring3 } openapiModelVersion.RegisteredModelId = (*source).RegisteredModelId pOpenapiModelVersion = &openapiModelVersion @@ -318,13 +315,10 @@ func (c *OpenAPIConverterImpl) ConvertRegisteredModelCreate(source *openapi.Regi xstring2 := *(*source).ExternalId openapiRegisteredModel.ExternalId = &xstring2 } - if (*source).Name != nil { - xstring3 := *(*source).Name - openapiRegisteredModel.Name = &xstring3 - } + openapiRegisteredModel.Name = (*source).Name if (*source).Owner != nil { - xstring4 := *(*source).Owner - openapiRegisteredModel.Owner = &xstring4 + xstring3 := *(*source).Owner + openapiRegisteredModel.Owner = &xstring3 } if (*source).State != nil { openapiRegisteredModelState, err := c.openapiRegisteredModelStateToOpenapiRegisteredModelState(*(*source).State) @@ -555,11 +549,10 @@ func (c *OpenAPIConverterImpl) OverrideNotEditableForModelVersion(source convert openapiModelVersion := converter.InitWithUpdate(source) var pString *string if source.Existing != nil { - pString = source.Existing.Name + pString = &source.Existing.Name } if pString != nil { - xstring := *pString - openapiModelVersion.Name = &xstring + openapiModelVersion.Name = *pString } var pString2 *string if source.Existing != nil { @@ -574,11 +567,10 @@ func (c *OpenAPIConverterImpl) OverrideNotEditableForRegisteredModel(source conv openapiRegisteredModel := converter.InitWithUpdate(source) var pString *string if source.Existing != nil { - pString = source.Existing.Name + pString = &source.Existing.Name } if pString != nil { - xstring := *pString - openapiRegisteredModel.Name = &xstring + openapiRegisteredModel.Name = *pString } return openapiRegisteredModel, nil } diff --git a/internal/converter/generated/openapi_mlmd_converter.gen.go b/internal/converter/generated/openapi_mlmd_converter.gen.go index 3626b05a..927d7e4c 100644 --- a/internal/converter/generated/openapi_mlmd_converter.gen.go +++ b/internal/converter/generated/openapi_mlmd_converter.gen.go @@ -233,7 +233,7 @@ func (c *OpenAPIToMLMDConverterImpl) ConvertRegisteredModel(source *converter.Op protoContext.Id = pInt64 var pString2 *string if (*source).Model != nil { - pString2 = (*source).Model.Name + pString2 = &(*source).Model.Name } if pString2 != nil { xstring := *pString2 diff --git a/internal/converter/mlmd_converter_util_test.go b/internal/converter/mlmd_converter_util_test.go index 86ccca2e..4d699b0f 100644 --- a/internal/converter/mlmd_converter_util_test.go +++ b/internal/converter/mlmd_converter_util_test.go @@ -216,7 +216,7 @@ func TestMapModelVersionProperties(t *testing.T) { ParentResourceId: of("123"), ModelName: of("MyModel"), Model: &openapi.ModelVersion{ - Name: of("v1"), + Name: "v1", Description: of("my model version description"), Author: of("John Doe"), }, @@ -244,7 +244,7 @@ func TestMapModelVersionName(t *testing.T) { ParentResourceId: of("123"), ModelName: of("MyModel"), Model: &openapi.ModelVersion{ - Name: of("v1"), + Name: "v1", }, }) assertion.NotNil(name) diff --git a/internal/converter/mlmd_openapi_converter.go b/internal/converter/mlmd_openapi_converter.go index 28f1dedc..21b2257b 100644 --- a/internal/converter/mlmd_openapi_converter.go +++ b/internal/converter/mlmd_openapi_converter.go @@ -19,7 +19,7 @@ type MLMDToOpenAPIConverter interface { // goverter:map Properties State | MapRegisteredModelState ConvertRegisteredModel(source *proto.Context) (*openapi.RegisteredModel, error) - // goverter:map Name | MapNameFromOwned + // goverter:map Name | MapName // goverter:map Name RegisteredModelId | MapRegisteredModelIdFromOwned // goverter:map Properties Description | MapDescription // goverter:map Properties State | MapModelVersionState diff --git a/internal/converter/mlmd_openapi_converter_util.go b/internal/converter/mlmd_openapi_converter_util.go index b0094469..9300f31d 100644 --- a/internal/converter/mlmd_openapi_converter_util.go +++ b/internal/converter/mlmd_openapi_converter_util.go @@ -100,6 +100,20 @@ func MapNameFromOwned(source *string) *string { return &exploded[1] } +// MapName derive the entity name from the mlmd fullname +// for owned entity such as ModelVersion +func MapName(source *string) string { + if source == nil { + return "" + } + + exploded := strings.Split(*source, ":") + if len(exploded) == 1 { + return *source + } + return exploded[1] +} + // REGISTERED MODEL // MODEL VERSION diff --git a/internal/converter/openapi_mlmd_converter.go b/internal/converter/openapi_mlmd_converter.go index ccf56fbe..3b6385c9 100644 --- a/internal/converter/openapi_mlmd_converter.go +++ b/internal/converter/openapi_mlmd_converter.go @@ -8,10 +8,10 @@ import ( type OpenAPIModelWrapper[ M OpenAPIModel, ] struct { - TypeId int64 Model *M - ParentResourceId *string // optional parent id - ModelName *string // optional registered model name + ParentResourceId *string + ModelName *string + TypeId int64 } // goverter:converter diff --git a/internal/converter/openapi_mlmd_converter_util.go b/internal/converter/openapi_mlmd_converter_util.go index ede00f3f..71c511f5 100644 --- a/internal/converter/openapi_mlmd_converter_util.go +++ b/internal/converter/openapi_mlmd_converter_util.go @@ -177,7 +177,7 @@ func MapModelVersionProperties(source *OpenAPIModelWrapper[openapi.ModelVersion] } props["version"] = &proto.Value{ Value: &proto.Value_StringValue{ - StringValue: *(*source.Model).Name, + StringValue: (*source.Model).Name, }, } @@ -208,7 +208,7 @@ func MapModelVersionType(_ *openapi.ModelVersion) *string { // MapModelVersionName maps the user-provided name into MLMD one, i.e., prefixing it with // either the parent resource id or a generated uuid func MapModelVersionName(source *OpenAPIModelWrapper[openapi.ModelVersion]) *string { - return of(PrefixWhenOwned(source.ParentResourceId, *(*source).Model.Name)) + return of(PrefixWhenOwned(source.ParentResourceId, (*source).Model.Name)) } // ARTIFACT diff --git a/internal/mapper/mapper.go b/internal/mapper/mapper.go index 9f28601a..ccbcf7fb 100644 --- a/internal/mapper/mapper.go +++ b/internal/mapper/mapper.go @@ -24,6 +24,11 @@ func NewMapper(mlmdTypes map[string]int64) *Mapper { } } +// of returns a pointer to the provided literal/const input +func of[E any](e E) *E { + return &e +} + // Utilities for OpenAPI --> MLMD mapping, make use of generated Converters func (m *Mapper) MapFromRegisteredModel(registeredModel *openapi.RegisteredModel) (*proto.Context, error) { @@ -33,12 +38,12 @@ func (m *Mapper) MapFromRegisteredModel(registeredModel *openapi.RegisteredModel }) } -func (m *Mapper) MapFromModelVersion(modelVersion *openapi.ModelVersion, registeredModelId string, registeredModelName *string) (*proto.Context, error) { +func (m *Mapper) MapFromModelVersion(modelVersion *openapi.ModelVersion, registeredModelId string, registeredModelName string) (*proto.Context, error) { return m.OpenAPIConverter.ConvertModelVersion(&converter.OpenAPIModelWrapper[openapi.ModelVersion]{ TypeId: m.MLMDTypes[defaults.ModelVersionTypeName], Model: modelVersion, ParentResourceId: ®isteredModelId, - ModelName: registeredModelName, + ModelName: of(registeredModelName), }) } diff --git a/internal/mapper/mapper_test.go b/internal/mapper/mapper_test.go index 04d55bbc..5f1b99be 100644 --- a/internal/mapper/mapper_test.go +++ b/internal/mapper/mapper_test.go @@ -40,7 +40,7 @@ func setup(t *testing.T) (*assert.Assertions, *Mapper) { func TestMapFromRegisteredModel(t *testing.T) { assertion, m := setup(t) - ctx, err := m.MapFromRegisteredModel(&openapi.RegisteredModel{Name: of("ModelName")}) + ctx, err := m.MapFromRegisteredModel(&openapi.RegisteredModel{Name: "ModelName"}) assertion.Nil(err) assertion.Equal("ModelName", ctx.GetName()) assertion.Equal(registeredModelTypeId, ctx.GetTypeId()) @@ -49,7 +49,7 @@ func TestMapFromRegisteredModel(t *testing.T) { func TestMapFromModelVersion(t *testing.T) { assertion, m := setup(t) - ctx, err := m.MapFromModelVersion(&openapi.ModelVersion{Name: of("v1")}, "1", of("ModelName")) + ctx, err := m.MapFromModelVersion(&openapi.ModelVersion{Name: "v1"}, "1", "ModelName") assertion.Nil(err) assertion.Equal("1:v1", ctx.GetName()) assertion.Equal(modelVersionTypeId, ctx.GetTypeId()) @@ -277,12 +277,7 @@ func TestMapToServeModelInvalid(t *testing.T) { } func TestMapTo(t *testing.T) { - _, err := mapTo[*proto.Execution, any](&proto.Execution{TypeId: of(registeredModelTypeId)}, typesMap, "notExisitingTypeName", func(e *proto.Execution) (*any, error) { return nil, nil }) + _, err := mapTo(&proto.Execution{TypeId: of(registeredModelTypeId)}, typesMap, "notExisitingTypeName", func(e *proto.Execution) (*any, error) { return nil, nil }) assert.NotNil(t, err) assert.Equal(t, "unknown type name provided: notExisitingTypeName", err.Error()) } - -// of returns a pointer to the provided literal/const input -func of[E any](e E) *E { - return &e -} diff --git a/internal/server/openapi/impl.go b/internal/server/openapi/impl.go index 0901dc5e..6ca6f2e8 100644 --- a/internal/server/openapi/impl.go +++ b/internal/server/openapi/impl.go @@ -11,6 +11,6 @@ package openapi // ImplResponse defines an implementation response with error code and the associated body type ImplResponse struct { - Code int Body interface{} + Code int } diff --git a/internal/server/openapi/type_asserts.go b/internal/server/openapi/type_asserts.go index 10a2f2e4..8defb4fc 100644 --- a/internal/server/openapi/type_asserts.go +++ b/internal/server/openapi/type_asserts.go @@ -524,6 +524,7 @@ func AssertModelArtifactUpdateConstraints(obj model.ModelArtifactUpdate) error { // AssertModelVersionRequired checks if the required fields are not zero-ed func AssertModelVersionRequired(obj model.ModelVersion) error { elements := map[string]interface{}{ + "name": obj.Name, "registeredModelId": obj.RegisteredModelId, } for name, el := range elements { @@ -543,6 +544,7 @@ func AssertModelVersionConstraints(obj model.ModelVersion) error { // AssertModelVersionCreateRequired checks if the required fields are not zero-ed func AssertModelVersionCreateRequired(obj model.ModelVersionCreate) error { elements := map[string]interface{}{ + "name": obj.Name, "registeredModelId": obj.RegisteredModelId, } for name, el := range elements { @@ -617,6 +619,15 @@ func AssertOrderByFieldConstraints(obj model.OrderByField) error { // AssertRegisteredModelRequired checks if the required fields are not zero-ed func AssertRegisteredModelRequired(obj model.RegisteredModel) error { + elements := map[string]interface{}{ + "name": obj.Name, + } + for name, el := range elements { + if isZero := IsZeroValue(el); isZero { + return &RequiredError{Field: name} + } + } + return nil } @@ -627,6 +638,15 @@ func AssertRegisteredModelConstraints(obj model.RegisteredModel) error { // AssertRegisteredModelCreateRequired checks if the required fields are not zero-ed func AssertRegisteredModelCreateRequired(obj model.RegisteredModelCreate) error { + elements := map[string]interface{}{ + "name": obj.Name, + } + for name, el := range elements { + if isZero := IsZeroValue(el); isZero { + return &RequiredError{Field: name} + } + } + return nil } diff --git a/pkg/core/core_test.go b/pkg/core/core_test.go index 89567511..ec729514 100644 --- a/pkg/core/core_test.go +++ b/pkg/core/core_test.go @@ -147,7 +147,7 @@ func (suite *CoreTestSuite) setupModelRegistryService() *ModelRegistryService { // utility function that register a new simple model and return its ID func (suite *CoreTestSuite) registerModel(service api.ModelRegistryApi, overrideModelName *string, overrideExternalId *string) string { registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, Description: &modelDescription, CustomProperties: &map[string]openapi.MetadataValue{ @@ -158,7 +158,7 @@ func (suite *CoreTestSuite) registerModel(service api.ModelRegistryApi, override } if overrideModelName != nil { - registeredModel.Name = overrideModelName + registeredModel.Name = *overrideModelName } if overrideExternalId != nil { @@ -213,14 +213,14 @@ func (suite *CoreTestSuite) registerModelVersion( registeredModelId := suite.registerModel(service, overrideModelName, overrideExternalId) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, Author: &author, } if overrideVersionName != nil { - modelVersion.Name = overrideVersionName + modelVersion.Name = *overrideVersionName } if overrideVersionExtId != nil { @@ -590,7 +590,7 @@ func (suite *CoreTestSuite) TestCreateRegisteredModel() { state := openapi.REGISTEREDMODELSTATE_ARCHIVED // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, Description: &modelDescription, Owner: &modelOwner, @@ -636,7 +636,7 @@ func (suite *CoreTestSuite) TestUpdateRegisteredModel() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, Owner: &modelOwner, ExternalId: &modelExternalId, CustomProperties: &map[string]openapi.MetadataValue{ @@ -655,7 +655,7 @@ func (suite *CoreTestSuite) TestUpdateRegisteredModel() { createdModelId, _ := converter.StringToInt64(createdModel.Id) // checks created model matches original one except for Id - suite.Equal(*registeredModel.Name, *createdModel.Name, "returned model name should match the original one") + suite.Equal(registeredModel.Name, createdModel.Name, "returned model name should match the original one") suite.Equal(*registeredModel.ExternalId, *createdModel.ExternalId, "returned model external id should match the original one") suite.Equal(*registeredModel.CustomProperties, *createdModel.CustomProperties, "returned model custom props should match the original one") @@ -700,7 +700,7 @@ func (suite *CoreTestSuite) TestUpdateRegisteredModel() { // update the model keeping nil name newModelExternalId = "newNewExternalId" createdModel.ExternalId = &newModelExternalId - createdModel.Name = nil + createdModel.Name = "" createdModel, err = service.UpsertRegisteredModel(createdModel) suite.Nilf(err, "error creating registered model: %v", err) @@ -731,7 +731,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelById() { state := openapi.REGISTEREDMODELSTATE_LIVE // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, State: &state, CustomProperties: &map[string]openapi.MetadataValue{ @@ -751,7 +751,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelById() { suite.Nilf(err, "error getting registered model by id %s: %v", *createdModel.Id, err) // checks created model matches original one except for Id - suite.Equal(*registeredModel.Name, *getModelById.Name, "saved model name should match the original one") + suite.Equal(registeredModel.Name, getModelById.Name, "saved model name should match the original one") suite.Equal(*registeredModel.ExternalId, *getModelById.ExternalId, "saved model external id should match the original one") suite.Equal(*registeredModel.State, *getModelById.State, "saved model state should match the original one") suite.Equal(*registeredModel.CustomProperties, *getModelById.CustomProperties, "saved model custom props should match the original one") @@ -772,7 +772,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelByParamsName() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -791,7 +791,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelByParamsExternalId() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -810,7 +810,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelByEmptyParams() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -830,7 +830,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsOrderedById() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -839,14 +839,14 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsOrderedById() { newModelName := "PricingModel2" newModelExternalId := "myExternalId2" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId _, err = service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) newModelName = "PricingModel3" newModelExternalId = "myExternalId3" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId _, err = service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) @@ -882,7 +882,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsOrderedByLastUpdate() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -891,14 +891,14 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsOrderedByLastUpdate() { newModelName := "PricingModel2" newModelExternalId := "myExternalId2" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId secondModel, err := service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) newModelName = "PricingModel3" newModelExternalId = "myExternalId3" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId thirdModel, err := service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) @@ -942,7 +942,7 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsWithPageSize() { // register a new model registeredModel := &openapi.RegisteredModel{ - Name: &modelName, + Name: modelName, ExternalId: &modelExternalId, } @@ -951,14 +951,14 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsWithPageSize() { newModelName := "PricingModel2" newModelExternalId := "myExternalId2" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId secondModel, err := service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) newModelName = "PricingModel3" newModelExternalId = "myExternalId3" - registeredModel.Name = &newModelName + registeredModel.Name = newModelName registeredModel.ExternalId = &newModelExternalId thirdModel, err := service.UpsertRegisteredModel(registeredModel) suite.Nilf(err, "error creating registered model: %v", err) @@ -994,7 +994,7 @@ func (suite *CoreTestSuite) TestCreateModelVersion() { state := openapi.MODELVERSIONSTATE_LIVE modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, State: &state, @@ -1037,7 +1037,7 @@ func (suite *CoreTestSuite) TestCreateModelVersionFailure() { registeredModelId := "9999" modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1058,7 +1058,7 @@ func (suite *CoreTestSuite) TestUpdateModelVersion() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1106,7 +1106,7 @@ func (suite *CoreTestSuite) TestUpdateModelVersion() { // update with nil name newExternalId = "org.my_awesome_model_@v1" updatedVersion.ExternalId = &newExternalId - updatedVersion.Name = nil + updatedVersion.Name = "" updatedVersion, err = service.UpsertModelVersion(updatedVersion, ®isteredModelId) suite.Nilf(err, "error updating new model version for %s: %v", registeredModelId, err) @@ -1136,7 +1136,7 @@ func (suite *CoreTestSuite) TestUpdateModelVersionFailure() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1168,7 +1168,7 @@ func (suite *CoreTestSuite) TestGetModelVersionById() { state := openapi.MODELVERSIONSTATE_ARCHIVED modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, State: &state, Author: &author, @@ -1192,7 +1192,7 @@ func (suite *CoreTestSuite) TestGetModelVersionById() { ctx := ctxById.Contexts[0] suite.Equal(*converter.Int64ToString(ctx.Id), *getById.Id, "returned model version id should match the mlmd context one") - suite.Equal(*modelVersion.Name, *getById.Name, "saved model name should match the provided one") + suite.Equal(modelVersion.Name, getById.Name, "saved model name should match the provided one") suite.Equal(*modelVersion.ExternalId, *getById.ExternalId, "saved external id should match the provided one") suite.Equal(*modelVersion.State, *getById.State, "saved model state should match the original one") suite.Equal(*getById.Author, author, "saved author property should match the provided one") @@ -1216,7 +1216,7 @@ func (suite *CoreTestSuite) TestGetModelVersionByParamsName() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1239,7 +1239,7 @@ func (suite *CoreTestSuite) TestGetModelVersionByParamsName() { ctx := ctxById.Contexts[0] suite.Equal(*converter.Int64ToString(ctx.Id), *getByName.Id, "returned model version id should match the mlmd context one") - suite.Equal(fmt.Sprintf("%s:%s", registeredModelId, *getByName.Name), *ctx.Name, "saved model name should match the provided one") + suite.Equal(fmt.Sprintf("%s:%s", registeredModelId, getByName.Name), *ctx.Name, "saved model name should match the provided one") suite.Equal(*ctx.ExternalId, *getByName.ExternalId, "saved external id should match the provided one") suite.Equal(ctx.Properties["author"].GetStringValue(), *getByName.Author, "saved author property should match the provided one") } @@ -1251,7 +1251,7 @@ func (suite *CoreTestSuite) TestGetModelVersionByParamsExternalId() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1274,7 +1274,7 @@ func (suite *CoreTestSuite) TestGetModelVersionByParamsExternalId() { ctx := ctxById.Contexts[0] suite.Equal(*converter.Int64ToString(ctx.Id), *getByExternalId.Id, "returned model version id should match the mlmd context one") - suite.Equal(fmt.Sprintf("%s:%s", registeredModelId, *getByExternalId.Name), *ctx.Name, "saved model name should match the provided one") + suite.Equal(fmt.Sprintf("%s:%s", registeredModelId, getByExternalId.Name), *ctx.Name, "saved model name should match the provided one") suite.Equal(*ctx.ExternalId, *getByExternalId.ExternalId, "saved external id should match the provided one") suite.Equal(ctx.Properties["author"].GetStringValue(), *getByExternalId.Author, "saved author property should match the provided one") } @@ -1286,7 +1286,7 @@ func (suite *CoreTestSuite) TestGetModelVersionByEmptyParams() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Author: &author, } @@ -1307,21 +1307,21 @@ func (suite *CoreTestSuite) TestGetModelVersions() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion1 := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, } secondModelVersionName := "v2" secondModelVersionExtId := "org.myawesomemodel@v2" modelVersion2 := &openapi.ModelVersion{ - Name: &secondModelVersionName, + Name: secondModelVersionName, ExternalId: &secondModelVersionExtId, } thirdModelVersionName := "v3" thirdModelVersionExtId := "org.myawesomemodel@v3" modelVersion3 := &openapi.ModelVersion{ - Name: &thirdModelVersionName, + Name: thirdModelVersionName, ExternalId: &thirdModelVersionExtId, } @@ -1341,7 +1341,7 @@ func (suite *CoreTestSuite) TestGetModelVersions() { anotherModelVersionName := "v1.0" anotherModelVersionExtId := "org.another@v1.0" modelVersionAnother := &openapi.ModelVersion{ - Name: &anotherModelVersionName, + Name: anotherModelVersionName, ExternalId: &anotherModelVersionExtId, } @@ -2641,13 +2641,13 @@ func (suite *CoreTestSuite) TestGetModelVersionByInferenceServiceId() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion1Name := "v1" - modelVersion1 := &openapi.ModelVersion{Name: &modelVersion1Name, Description: &modelVersionDescription} + modelVersion1 := &openapi.ModelVersion{Name: modelVersion1Name, Description: &modelVersionDescription} createdVersion1, err := service.UpsertModelVersion(modelVersion1, ®isteredModelId) suite.Nilf(err, "error creating new model version for %d", registeredModelId) createdVersion1Id := *createdVersion1.Id modelVersion2Name := "v2" - modelVersion2 := &openapi.ModelVersion{Name: &modelVersion2Name, Description: &modelVersionDescription} + modelVersion2 := &openapi.ModelVersion{Name: modelVersion2Name, Description: &modelVersionDescription} createdVersion2, err := service.UpsertModelVersion(modelVersion2, ®isteredModelId) suite.Nilf(err, "error creating new model version for %d", registeredModelId) createdVersion2Id := *createdVersion2.Id @@ -2691,7 +2691,7 @@ func (suite *CoreTestSuite) TestGetModelArtifactByInferenceServiceId() { registeredModelId := suite.registerModel(service, nil, nil) modelVersion1Name := "v1" - modelVersion1 := &openapi.ModelVersion{Name: &modelVersion1Name, Description: &modelVersionDescription} + modelVersion1 := &openapi.ModelVersion{Name: modelVersion1Name, Description: &modelVersionDescription} createdVersion1, err := service.UpsertModelVersion(modelVersion1, ®isteredModelId) suite.Nilf(err, "error creating new model version for %s", registeredModelId) modelArtifact1Name := "v1-artifact" @@ -2700,7 +2700,7 @@ func (suite *CoreTestSuite) TestGetModelArtifactByInferenceServiceId() { suite.Nilf(err, "error creating new model artifact for %s", *createdVersion1.Id) modelVersion2Name := "v2" - modelVersion2 := &openapi.ModelVersion{Name: &modelVersion2Name, Description: &modelVersionDescription} + modelVersion2 := &openapi.ModelVersion{Name: modelVersion2Name, Description: &modelVersionDescription} createdVersion2, err := service.UpsertModelVersion(modelVersion2, ®isteredModelId) suite.Nilf(err, "error creating new model version for %s", registeredModelId) modelArtifact2Name := "v2-artifact" @@ -2988,7 +2988,7 @@ func (suite *CoreTestSuite) TestCreateServeModel() { inferenceServiceId := suite.registerInferenceService(service, registeredModelId, nil, nil, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, Author: &author, @@ -3082,7 +3082,7 @@ func (suite *CoreTestSuite) TestUpdateServeModel() { inferenceServiceId := suite.registerInferenceService(service, registeredModelId, nil, nil, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, Author: &author, @@ -3144,7 +3144,7 @@ func (suite *CoreTestSuite) TestUpdateServeModelFailure() { inferenceServiceId := suite.registerInferenceService(service, registeredModelId, nil, nil, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, Author: &author, @@ -3191,7 +3191,7 @@ func (suite *CoreTestSuite) TestGetServeModelById() { inferenceServiceId := suite.registerInferenceService(service, registeredModelId, nil, nil, nil, nil) modelVersion := &openapi.ModelVersion{ - Name: &modelVersionName, + Name: modelVersionName, ExternalId: &versionExternalId, Description: &modelVersionDescription, Author: &author, @@ -3238,19 +3238,19 @@ func (suite *CoreTestSuite) TestGetServeModels() { inferenceServiceId := suite.registerInferenceService(service, registeredModelId, nil, nil, nil, nil) modelVersion1Name := "v1" - modelVersion1 := &openapi.ModelVersion{Name: &modelVersion1Name, Description: &modelVersionDescription} + modelVersion1 := &openapi.ModelVersion{Name: modelVersion1Name, Description: &modelVersionDescription} createdVersion1, err := service.UpsertModelVersion(modelVersion1, ®isteredModelId) suite.Nilf(err, "error creating new model version for %d", registeredModelId) createdVersion1Id := *createdVersion1.Id modelVersion2Name := "v2" - modelVersion2 := &openapi.ModelVersion{Name: &modelVersion2Name, Description: &modelVersionDescription} + modelVersion2 := &openapi.ModelVersion{Name: modelVersion2Name, Description: &modelVersionDescription} createdVersion2, err := service.UpsertModelVersion(modelVersion2, ®isteredModelId) suite.Nilf(err, "error creating new model version for %d", registeredModelId) createdVersion2Id := *createdVersion2.Id modelVersion3Name := "v3" - modelVersion3 := &openapi.ModelVersion{Name: &modelVersion3Name, Description: &modelVersionDescription} + modelVersion3 := &openapi.ModelVersion{Name: modelVersion3Name, Description: &modelVersionDescription} createdVersion3, err := service.UpsertModelVersion(modelVersion3, ®isteredModelId) suite.Nilf(err, "error creating new model version for %d", registeredModelId) createdVersion3Id := *createdVersion3.Id diff --git a/pkg/openapi/model_model_version.go b/pkg/openapi/model_model_version.go index c0b89f8c..01f92277 100644 --- a/pkg/openapi/model_model_version.go +++ b/pkg/openapi/model_model_version.go @@ -26,7 +26,7 @@ type ModelVersion struct { // The external id that come from the clients’ system. This field is optional. If set, it must be unique among all resources within a database instance. ExternalId *string `json:"externalId,omitempty"` // The client provided name of the artifact. This field is optional. If set, it must be unique among all the artifacts of the same artifact type within a database instance and cannot be changed once set. - Name *string `json:"name,omitempty"` + Name string `json:"name"` State *ModelVersionState `json:"state,omitempty"` // Name of the author. Author *string `json:"author,omitempty"` @@ -44,8 +44,9 @@ type ModelVersion struct { // This constructor will assign default values to properties that have it defined, // and makes sure properties required by API are set, but the set of arguments // will change when the set of required properties is changed -func NewModelVersion(registeredModelId string) *ModelVersion { +func NewModelVersion(name string, registeredModelId string) *ModelVersion { this := ModelVersion{} + this.Name = name var state ModelVersionState = MODELVERSIONSTATE_LIVE this.State = &state this.RegisteredModelId = registeredModelId @@ -158,36 +159,28 @@ func (o *ModelVersion) SetExternalId(v string) { o.ExternalId = &v } -// GetName returns the Name field value if set, zero value otherwise. +// GetName returns the Name field value func (o *ModelVersion) GetName() string { - if o == nil || IsNil(o.Name) { + if o == nil { var ret string return ret } - return *o.Name + + return o.Name } -// GetNameOk returns a tuple with the Name field value if set, nil otherwise +// GetNameOk returns a tuple with the Name field value // and a boolean to check if the value has been set. func (o *ModelVersion) GetNameOk() (*string, bool) { - if o == nil || IsNil(o.Name) { + if o == nil { return nil, false } - return o.Name, true -} - -// HasName returns a boolean if a field has been set. -func (o *ModelVersion) HasName() bool { - if o != nil && !IsNil(o.Name) { - return true - } - - return false + return &o.Name, true } -// SetName gets a reference to the given string and assigns it to the Name field. +// SetName sets field value func (o *ModelVersion) SetName(v string) { - o.Name = &v + o.Name = v } // GetState returns the State field value if set, zero value otherwise. @@ -393,9 +386,7 @@ func (o ModelVersion) ToMap() (map[string]interface{}, error) { if !IsNil(o.ExternalId) { toSerialize["externalId"] = o.ExternalId } - if !IsNil(o.Name) { - toSerialize["name"] = o.Name - } + toSerialize["name"] = o.Name if !IsNil(o.State) { toSerialize["state"] = o.State } diff --git a/pkg/openapi/model_model_version_create.go b/pkg/openapi/model_model_version_create.go index e964055a..5a6c743a 100644 --- a/pkg/openapi/model_model_version_create.go +++ b/pkg/openapi/model_model_version_create.go @@ -25,8 +25,8 @@ type ModelVersionCreate struct { Description *string `json:"description,omitempty"` // The external id that come from the clients’ system. This field is optional. If set, it must be unique among all resources within a database instance. ExternalId *string `json:"externalId,omitempty"` - // The client provided name of the artifact. This field is optional. If set, it must be unique among all the artifacts of the same artifact type within a database instance and cannot be changed once set. - Name *string `json:"name,omitempty"` + // The client provided name of the model's version. It must be unique among all the ModelVersions of the same type within a Model Registry instance and cannot be changed once set. + Name string `json:"name"` State *ModelVersionState `json:"state,omitempty"` // Name of the author. Author *string `json:"author,omitempty"` @@ -38,8 +38,9 @@ type ModelVersionCreate struct { // This constructor will assign default values to properties that have it defined, // and makes sure properties required by API are set, but the set of arguments // will change when the set of required properties is changed -func NewModelVersionCreate(registeredModelId string) *ModelVersionCreate { +func NewModelVersionCreate(name string, registeredModelId string) *ModelVersionCreate { this := ModelVersionCreate{} + this.Name = name var state ModelVersionState = MODELVERSIONSTATE_LIVE this.State = &state this.RegisteredModelId = registeredModelId @@ -152,36 +153,28 @@ func (o *ModelVersionCreate) SetExternalId(v string) { o.ExternalId = &v } -// GetName returns the Name field value if set, zero value otherwise. +// GetName returns the Name field value func (o *ModelVersionCreate) GetName() string { - if o == nil || IsNil(o.Name) { + if o == nil { var ret string return ret } - return *o.Name + + return o.Name } -// GetNameOk returns a tuple with the Name field value if set, nil otherwise +// GetNameOk returns a tuple with the Name field value // and a boolean to check if the value has been set. func (o *ModelVersionCreate) GetNameOk() (*string, bool) { - if o == nil || IsNil(o.Name) { + if o == nil { return nil, false } - return o.Name, true -} - -// HasName returns a boolean if a field has been set. -func (o *ModelVersionCreate) HasName() bool { - if o != nil && !IsNil(o.Name) { - return true - } - - return false + return &o.Name, true } -// SetName gets a reference to the given string and assigns it to the Name field. +// SetName sets field value func (o *ModelVersionCreate) SetName(v string) { - o.Name = &v + o.Name = v } // GetState returns the State field value if set, zero value otherwise. @@ -291,9 +284,7 @@ func (o ModelVersionCreate) ToMap() (map[string]interface{}, error) { if !IsNil(o.ExternalId) { toSerialize["externalId"] = o.ExternalId } - if !IsNil(o.Name) { - toSerialize["name"] = o.Name - } + toSerialize["name"] = o.Name if !IsNil(o.State) { toSerialize["state"] = o.State } diff --git a/pkg/openapi/model_registered_model.go b/pkg/openapi/model_registered_model.go index 73ce09a6..a692a149 100644 --- a/pkg/openapi/model_registered_model.go +++ b/pkg/openapi/model_registered_model.go @@ -26,7 +26,7 @@ type RegisteredModel struct { // The external id that come from the clients’ system. This field is optional. If set, it must be unique among all resources within a database instance. ExternalId *string `json:"externalId,omitempty"` // The client provided name of the artifact. This field is optional. If set, it must be unique among all the artifacts of the same artifact type within a database instance and cannot be changed once set. - Name *string `json:"name,omitempty"` + Name string `json:"name"` // Output only. The unique server generated id of the resource. Id *string `json:"id,omitempty"` // Output only. Create time of the resource in millisecond since epoch. @@ -41,8 +41,9 @@ type RegisteredModel struct { // This constructor will assign default values to properties that have it defined, // and makes sure properties required by API are set, but the set of arguments // will change when the set of required properties is changed -func NewRegisteredModel() *RegisteredModel { +func NewRegisteredModel(name string) *RegisteredModel { this := RegisteredModel{} + this.Name = name var state RegisteredModelState = REGISTEREDMODELSTATE_LIVE this.State = &state return &this @@ -154,36 +155,28 @@ func (o *RegisteredModel) SetExternalId(v string) { o.ExternalId = &v } -// GetName returns the Name field value if set, zero value otherwise. +// GetName returns the Name field value func (o *RegisteredModel) GetName() string { - if o == nil || IsNil(o.Name) { + if o == nil { var ret string return ret } - return *o.Name + + return o.Name } -// GetNameOk returns a tuple with the Name field value if set, nil otherwise +// GetNameOk returns a tuple with the Name field value // and a boolean to check if the value has been set. func (o *RegisteredModel) GetNameOk() (*string, bool) { - if o == nil || IsNil(o.Name) { + if o == nil { return nil, false } - return o.Name, true -} - -// HasName returns a boolean if a field has been set. -func (o *RegisteredModel) HasName() bool { - if o != nil && !IsNil(o.Name) { - return true - } - - return false + return &o.Name, true } -// SetName gets a reference to the given string and assigns it to the Name field. +// SetName sets field value func (o *RegisteredModel) SetName(v string) { - o.Name = &v + o.Name = v } // GetId returns the Id field value if set, zero value otherwise. @@ -365,9 +358,7 @@ func (o RegisteredModel) ToMap() (map[string]interface{}, error) { if !IsNil(o.ExternalId) { toSerialize["externalId"] = o.ExternalId } - if !IsNil(o.Name) { - toSerialize["name"] = o.Name - } + toSerialize["name"] = o.Name if !IsNil(o.Id) { toSerialize["id"] = o.Id } diff --git a/pkg/openapi/model_registered_model_create.go b/pkg/openapi/model_registered_model_create.go index cf6df186..49edf663 100644 --- a/pkg/openapi/model_registered_model_create.go +++ b/pkg/openapi/model_registered_model_create.go @@ -26,7 +26,7 @@ type RegisteredModelCreate struct { // The external id that come from the clients’ system. This field is optional. If set, it must be unique among all resources within a database instance. ExternalId *string `json:"externalId,omitempty"` // The client provided name of the artifact. This field is optional. If set, it must be unique among all the artifacts of the same artifact type within a database instance and cannot be changed once set. - Name *string `json:"name,omitempty"` + Name string `json:"name"` Owner *string `json:"owner,omitempty"` State *RegisteredModelState `json:"state,omitempty"` } @@ -35,8 +35,9 @@ type RegisteredModelCreate struct { // This constructor will assign default values to properties that have it defined, // and makes sure properties required by API are set, but the set of arguments // will change when the set of required properties is changed -func NewRegisteredModelCreate() *RegisteredModelCreate { +func NewRegisteredModelCreate(name string) *RegisteredModelCreate { this := RegisteredModelCreate{} + this.Name = name var state RegisteredModelState = REGISTEREDMODELSTATE_LIVE this.State = &state return &this @@ -148,36 +149,28 @@ func (o *RegisteredModelCreate) SetExternalId(v string) { o.ExternalId = &v } -// GetName returns the Name field value if set, zero value otherwise. +// GetName returns the Name field value func (o *RegisteredModelCreate) GetName() string { - if o == nil || IsNil(o.Name) { + if o == nil { var ret string return ret } - return *o.Name + + return o.Name } -// GetNameOk returns a tuple with the Name field value if set, nil otherwise +// GetNameOk returns a tuple with the Name field value // and a boolean to check if the value has been set. func (o *RegisteredModelCreate) GetNameOk() (*string, bool) { - if o == nil || IsNil(o.Name) { + if o == nil { return nil, false } - return o.Name, true -} - -// HasName returns a boolean if a field has been set. -func (o *RegisteredModelCreate) HasName() bool { - if o != nil && !IsNil(o.Name) { - return true - } - - return false + return &o.Name, true } -// SetName gets a reference to the given string and assigns it to the Name field. +// SetName sets field value func (o *RegisteredModelCreate) SetName(v string) { - o.Name = &v + o.Name = v } // GetOwner returns the Owner field value if set, zero value otherwise. @@ -263,9 +256,7 @@ func (o RegisteredModelCreate) ToMap() (map[string]interface{}, error) { if !IsNil(o.ExternalId) { toSerialize["externalId"] = o.ExternalId } - if !IsNil(o.Name) { - toSerialize["name"] = o.Name - } + toSerialize["name"] = o.Name if !IsNil(o.Owner) { toSerialize["owner"] = o.Owner } diff --git a/test/robot/Regression.robot b/test/robot/Regression.robot index fed9d76b..0cbf899a 100644 --- a/test/robot/Regression.robot +++ b/test/robot/Regression.robot @@ -14,3 +14,10 @@ As a MLOps engineer if I try to store a malformed RegisteredModel I get a struct ${err} POST url=http://${MR_HOST}:${MR_PORT}/api/model_registry/v1alpha3/registered_models json=&{rm} expected_status=400 ${rm_err} Create Dictionary code= message=json: unknown field "ext_id" And Should be equal ${rm_err} ${err.json()} + +As a MLOps engineer if I try to store a malformed ModelVersion I get a structured error message + ${rId} Given I create a RegisteredModel having name=${name} + ${mv} Create Dictionary registeredModelId=${rId} + ${err} POST url=http://${MR_HOST}:${MR_PORT}/api/model_registry/v1alpha3/model_versions json=&{mv} expected_status=422 + ${mv_err} Create Dictionary code= message=required field 'name' is zero value. + And Should be equal ${mv_err} ${err.json()}