Skip to content

Commit

Permalink
add DocArtifact type
Browse files Browse the repository at this point in the history
Closes: kubeflow#263

Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
  • Loading branch information
isinyaaa committed Feb 9, 2024
1 parent cf1345a commit 70eefeb
Show file tree
Hide file tree
Showing 24 changed files with 1,320 additions and 179 deletions.
15 changes: 13 additions & 2 deletions api/openapi/model-registry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,7 @@ paths:
"500":
$ref: "#/components/responses/InternalServerError"
operationId: getModelVersionArtifacts
summary: List All ModelVersion's artifacts
description: Gets a list of all `Artifact` entities for the `ModelVersion`.
summary: List all artifacts associated with the `ModelVersion`
post:
requestBody:
description: A new or existing `Artifact` to be associated with the `ModelVersion`.
Expand Down Expand Up @@ -979,6 +978,16 @@ components:
artifactType:
type: string
default: "model-artifact"
DocArtifact:
description: A document.
type: object
allOf:
- $ref: "#/components/schemas/BaseArtifact"
- type: object
properties:
artifactType:
type: string
default: "doc-artifact"
RegisteredModel:
description: A registered model in model registry. A registered model has ModelVersion children.
allOf:
Expand Down Expand Up @@ -1272,10 +1281,12 @@ components:
Artifact:
oneOf:
- $ref: "#/components/schemas/ModelArtifact"
- $ref: "#/components/schemas/DocArtifact"
discriminator:
propertyName: artifactType
mapping:
model-artifact: "#/components/schemas/ModelArtifact"
doc-artifact: "#/components/schemas/DocArtifact"
description: A metadata Artifact Entity.
BaseArtifact:
allOf:
Expand Down
1 change: 1 addition & 0 deletions internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const (
RegisteredModelTypeName = "odh.RegisteredModel"
ModelVersionTypeName = "odh.ModelVersion"
ModelArtifactTypeName = "odh.ModelArtifact"
DocArtifactTypeName = "odh.DocArtifact"
ServingEnvironmentTypeName = "odh.ServingEnvironment"
InferenceServiceTypeName = "odh.InferenceService"
ServeModelTypeName = "odh.ServeModel"
Expand Down
36 changes: 36 additions & 0 deletions internal/converter/generated/mlmd_openapi_converter.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions internal/converter/generated/openapi_converter.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 64 additions & 0 deletions internal/converter/generated/openapi_mlmd_converter.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 65 additions & 0 deletions internal/converter/mlmd_converter_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,65 @@ func TestMapModelArtifactName(t *testing.T) {
assertion.Regexp(".*:v1", *name)
}

func TestMapDocArtifactProperties(t *testing.T) {
assertion := setup(t)

props, err := MapDocArtifactProperties(&openapi.DocArtifact{
Name: of("v1"),
Description: of("my model art description"),
})
assertion.Nil(err)
assertion.Equal(1, len(props))
assertion.Equal("my model art description", props["description"].GetStringValue())

props, err = MapModelArtifactProperties(&openapi.ModelArtifact{
Name: of("v1"),
})
assertion.Nil(err)
assertion.Equal(0, len(props))
}

func TestMapDocArtifactType(t *testing.T) {
assertion := setup(t)

typeName := MapModelArtifactType(&openapi.ModelArtifact{})
assertion.NotNil(typeName)
assertion.Equal(constants.ModelArtifactTypeName, *typeName)
}

func TestMapDocArtifactName(t *testing.T) {
assertion := setup(t)

name := MapDocArtifactName(&OpenAPIModelWrapper[openapi.DocArtifact]{
TypeId: 123,
ParentResourceId: of("parent"),
Model: &openapi.DocArtifact{
Name: of("v1"),
},
})
assertion.NotNil(name)
assertion.Equal("parent:v1", *name)

name = MapDocArtifactName(&OpenAPIModelWrapper[openapi.DocArtifact]{
TypeId: 123,
ParentResourceId: of("parent"),
Model: &openapi.DocArtifact{
Name: nil,
},
})
assertion.NotNil(name)
assertion.Regexp("parent:.*", *name)

name = MapDocArtifactName(&OpenAPIModelWrapper[openapi.DocArtifact]{
TypeId: 123,
Model: &openapi.DocArtifact{
Name: of("v1"),
},
})
assertion.NotNil(name)
assertion.Regexp(".*:v1", *name)
}

func TestMapOpenAPIArtifactState(t *testing.T) {
assertion := setup(t)

Expand Down Expand Up @@ -523,6 +582,12 @@ func TestMapArtifactType(t *testing.T) {
assertion.Nil(err)
assertion.Equal("model-artifact", artifactType)

artifactType, err = MapArtifactType(&proto.Artifact{
Type: of(constants.DocArtifactTypeName),
})
assertion.Nil(err)
assertion.Equal("doc-artifact", artifactType)

artifactType, err = MapArtifactType(&proto.Artifact{
Type: of("Invalid"),
})
Expand Down
6 changes: 6 additions & 0 deletions internal/converter/mlmd_openapi_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ type MLMDToOpenAPIConverter interface {
// goverter:map Properties ServiceAccountName | MapModelArtifactServiceAccountName
ConvertModelArtifact(source *proto.Artifact) (*openapi.ModelArtifact, error)

// goverter:map Name | MapNameFromOwned
// goverter:map . ArtifactType | MapArtifactType
// goverter:map State | MapMLMDArtifactState
// goverter:map Properties Description | MapDescription
ConvertDocArtifact(source *proto.Artifact) (*openapi.DocArtifact, error)

// goverter:map Name | MapNameFromOwned
// goverter:map Properties Description | MapDescription
ConvertServingEnvironment(source *proto.Context) (*openapi.ServingEnvironment, error)
Expand Down
15 changes: 12 additions & 3 deletions internal/converter/mlmd_openapi_converter_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,24 @@ func MapPropertyAuthor(properties map[string]*proto.Value) *string {
return MapStringProperty(properties, "author")
}

// MODEL ARTIFACT
// ARTIFACT

func MapArtifactType(source *proto.Artifact) (string, error) {
if source.Type != nil && *source.Type == constants.ModelArtifactTypeName {
if source.Type == nil {
return "", fmt.Errorf("artifact type is nil")
}
switch *source.Type {
case constants.ModelArtifactTypeName:
return "model-artifact", nil
case constants.DocArtifactTypeName:
return "doc-artifact", nil
default:
return "", fmt.Errorf("invalid artifact type found: %v", source.Type)
}
return "", fmt.Errorf("invalid artifact type found: %v", source.Type)
}

// MODEL ARTIFACT

func MapRegisteredModelState(properties map[string]*proto.Value) *openapi.RegisteredModelState {
state, ok := properties["state"]
if !ok {
Expand Down
6 changes: 6 additions & 0 deletions internal/converter/openapi_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ type OpenAPIConverter interface {
// goverter:ignore Id CreateTimeSinceEpoch LastUpdateTimeSinceEpoch Description ExternalID CustomProperties State Author
OverrideNotEditableForModelVersion(source OpenapiUpdateWrapper[openapi.ModelVersion]) (openapi.ModelVersion, error)

// Ignore all fields that ARE editable
// goverter:default InitDocArtifactWithUpdate
// goverter:autoMap Existing
// goverter:ignore Id Name ArtifactType CreateTimeSinceEpoch LastUpdateTimeSinceEpoch Description ExternalID CustomProperties Uri State
OverrideNotEditableForDocArtifact(source OpenapiUpdateWrapper[openapi.DocArtifact]) (openapi.DocArtifact, error)

// Ignore all fields that ARE editable
// goverter:default InitModelArtifactWithUpdate
// goverter:autoMap Existing
Expand Down
3 changes: 3 additions & 0 deletions internal/converter/openapi_converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ func newVisitor(t *testing.T, f *ast.File) visitor {
"ModelVersion": {
obj: openapi.ModelVersion{},
},
"DocArtifact": {
obj: openapi.DocArtifact{},
},
"ModelArtifact": {
obj: openapi.ModelArtifact{},
},
Expand Down
8 changes: 8 additions & 0 deletions internal/converter/openapi_converter_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type OpenAPIModel interface {
openapi.RegisteredModel |
openapi.ModelVersion |
openapi.ModelArtifact |
openapi.DocArtifact |
openapi.ServingEnvironment |
openapi.InferenceService |
openapi.ServeModel
Expand Down Expand Up @@ -41,6 +42,13 @@ func InitModelVersionWithUpdate(source OpenapiUpdateWrapper[openapi.ModelVersion
return openapi.ModelVersion{}
}

func InitDocArtifactWithUpdate(source OpenapiUpdateWrapper[openapi.DocArtifact]) openapi.DocArtifact {
if source.Update != nil {
return *source.Update
}
return openapi.DocArtifact{}
}

func InitModelArtifactWithUpdate(source OpenapiUpdateWrapper[openapi.ModelArtifact]) openapi.ModelArtifact {
if source.Update != nil {
return *source.Update
Expand Down
8 changes: 8 additions & 0 deletions internal/converter/openapi_mlmd_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ type OpenAPIToMLMDConverter interface {
// goverter:ignore state sizeCache unknownFields SystemMetadata CreateTimeSinceEpoch LastUpdateTimeSinceEpoch
ConvertModelArtifact(source *OpenAPIModelWrapper[openapi.ModelArtifact]) (*proto.Artifact, error)

// goverter:autoMap Model
// goverter:map . Name | MapDocArtifactName
// goverter:map Model Type | MapDocArtifactType
// goverter:map Model Properties | MapDocArtifactProperties
// goverter:map Model.State State | MapOpenAPIArtifactState
// goverter:ignore state sizeCache unknownFields SystemMetadata CreateTimeSinceEpoch LastUpdateTimeSinceEpoch
ConvertDocArtifact(source *OpenAPIModelWrapper[openapi.DocArtifact]) (*proto.Artifact, error)

// goverter:autoMap Model
// goverter:map Model Type | MapServingEnvironmentType
// goverter:map Model Properties | MapServingEnvironmentProperties
Expand Down
35 changes: 35 additions & 0 deletions internal/converter/openapi_mlmd_converter_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,41 @@ func MapOpenAPIArtifactState(source *openapi.ArtifactState) (*proto.Artifact_Sta
return (*proto.Artifact_State)(&val), nil
}

// DOC ARTIFACT

// get DocArtifact MLMD type name
func MapDocArtifactType(_ *openapi.DocArtifact) *string {
return of(constants.DocArtifactTypeName)
}

func MapDocArtifactProperties(source *openapi.DocArtifact) (map[string]*proto.Value, error) {
props := make(map[string]*proto.Value)
if source == nil {
return nil, nil
}
if source.Description != nil {
props["description"] = &proto.Value{
Value: &proto.Value_StringValue{
StringValue: *source.Description,
},
}
}
return props, nil
}

// maps the user-provided name into MLMD one, i.e., prefixing it with either the parent resource id or a generated
// uuid. If not provided, autogenerate the name itself
func MapDocArtifactName(source *OpenAPIModelWrapper[openapi.DocArtifact]) *string {
// openapi.Artifact is defined with optional name, so build arbitrary name for this artifact if missing
var artifactName string
if (*source).Model.Name != nil {
artifactName = *(*source).Model.Name
} else {
artifactName = uuid.New().String()
}
return of(PrefixWhenOwned(source.ParentResourceId, artifactName))
}

// MODEL ARTIFACT

// MapModelArtifactProperties maps ModelArtifact fields to specific MLMD properties
Expand Down
Loading

0 comments on commit 70eefeb

Please sign in to comment.