OAS: Fix discriminator field definition for Artifact (#22)

* gitignore: ignore all coverage files Signed-off-by: Isabella Basso do Amaral <[email protected]> * OAS: fix discriminator field for Artifact Signed-off-by: Isabella Basso do Amaral <[email protected]> --------- Signed-off-by: Isabella Basso do Amaral <[email protected]>
kubeflow · Mar 1, 2024 · be3b275 · be3b275
1 parent 247c23c
commit be3b275
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 181 deletions.
diff --git a/.gitignore b/.gitignore
@@ -27,7 +27,7 @@ metadata.sqlite.db
 
 # Ignore go vendor and code coverage files
 vendor
-coverage.txt
+coverage.*
 
 # Robot Framework files
 log.html

diff --git a/api/openapi/model-registry.yaml b/api/openapi/model-registry.yaml
@@ -971,24 +971,26 @@ components:
     ModelArtifact:
       description: An ML model artifact.
       type: object
+      required:
+        - artifactType
+      properties:
+        artifactType:
+          type: string
+          default: "model-artifact"
       allOf:
         - $ref: "#/components/schemas/BaseArtifact"
         - $ref: "#/components/schemas/ModelArtifactCreate"
-        - type: object
-          properties:
-            artifactType:
-              type: string
-              default: "model-artifact"
     DocArtifact:
       description: A document.
       type: object
+      required:
+        - artifactType
+      properties:
+        artifactType:
+          type: string
+          default: "doc-artifact"
       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:
@@ -1105,12 +1107,12 @@ components:
       discriminator:
         propertyName: metadataType
         mapping:
-            MetadataBoolValue: '#/components/schemas/MetadataBoolValue'
-            MetadataDoubleValue: '#/components/schemas/MetadataDoubleValue'
-            MetadataIntValue: '#/components/schemas/MetadataIntValue'
-            MetadataProtoValue: '#/components/schemas/MetadataProtoValue'
-            MetadataStringValue: '#/components/schemas/MetadataStringValue'
-            MetadataStructValue: '#/components/schemas/MetadataStructValue'
+          MetadataBoolValue: "#/components/schemas/MetadataBoolValue"
+          MetadataDoubleValue: "#/components/schemas/MetadataDoubleValue"
+          MetadataIntValue: "#/components/schemas/MetadataIntValue"
+          MetadataProtoValue: "#/components/schemas/MetadataProtoValue"
+          MetadataStringValue: "#/components/schemas/MetadataStringValue"
+          MetadataStructValue: "#/components/schemas/MetadataStructValue"
       description: A value in properties.
     MetadataIntValue:
       description: An integer (int64) property value.
@@ -1345,12 +1347,6 @@ components:
       allOf:
         - $ref: "#/components/schemas/BaseArtifactCreate"
         - $ref: "#/components/schemas/BaseResource"
-        - required:
-            - artifactType
-          type: object
-          properties:
-            artifactType:
-              type: string
     ServingEnvironmentList:
       description: List of ServingEnvironments.
       type: object

diff --git a/internal/converter/generated/mlmd_openapi_converter.gen.go b/internal/converter/generated/mlmd_openapi_converter.gen.go
diff --git a/internal/converter/generated/openapi_converter.gen.go b/internal/converter/generated/openapi_converter.gen.go
diff --git a/internal/server/openapi/type_asserts.go b/internal/server/openapi/type_asserts.go
@@ -18,15 +18,6 @@ import (
 
 // AssertArtifactRequired checks if the required fields are not zero-ed
 func AssertArtifactRequired(obj model.Artifact) error {
-	// FIXME(manual): Artifact.ArtifactType is not present on client models
-	// elements := map[string]interface{}{
-	// 	"artifactType": obj.ArtifactType,
-	// }
-	// for name, el := range elements {
-	// 	if isZero := IsZeroValue(el); isZero {
-	// 		return &RequiredError{Field: name}
-	// 	}
-	// }
 	return nil
 }
 
@@ -83,15 +74,6 @@ func AssertBaseArtifactCreateConstraints(obj model.BaseArtifactCreate) error {
 
 // AssertBaseArtifactRequired checks if the required fields are not zero-ed
 func AssertBaseArtifactRequired(obj model.BaseArtifact) error {
-	elements := map[string]interface{}{
-		"artifactType": obj.ArtifactType,
-	}
-	for name, el := range elements {
-		if isZero := IsZeroValue(el); isZero {
-			return &RequiredError{Field: name}
-		}
-	}
-
 	return nil
 }
 

diff --git a/patches/type_asserts.patch b/patches/type_asserts.patch
@@ -2,31 +2,6 @@ diff --git a/internal/server/openapi/type_asserts.go b/internal/server/openapi/t
 index b001018..9907fbc 100644
 --- a/internal/server/openapi/type_asserts.go
 +++ b/internal/server/openapi/type_asserts.go
-@@ -18,15 +18,15 @@ import (
-
- // AssertArtifactRequired checks if the required fields are not zero-ed
- func AssertArtifactRequired(obj model.Artifact) error {
--	elements := map[string]interface{}{
--		"artifactType": obj.ArtifactType,
--	}
--	for name, el := range elements {
--		if isZero := IsZeroValue(el); isZero {
--			return &RequiredError{Field: name}
--		}
--	}
--
-+	// FIXME(manual): Artifact.ArtifactType is not present on client models
-+	// elements := map[string]interface{}{
-+	// 	"artifactType": obj.ArtifactType,
-+	// }
-+	// for name, el := range elements {
-+	// 	if isZero := IsZeroValue(el); isZero {
-+	// 		return &RequiredError{Field: name}
-+	// 	}
-+	// }
- 	return nil
- }
-
 @@ -449,21 +449,22 @@ func AssertMetadataStructValueConstraints(obj model.MetadataStructValue) error {
 
  // AssertMetadataValueRequired checks if the required fields are not zero-ed

diff --git a/pkg/openapi/model_base_artifact.go b/pkg/openapi/model_base_artifact.go