core: openapi factories, do not require artifactType (#538)

* core: make server generation use default factories ..introducing those missing. Signed-off-by: Matteo Mortari <[email protected]> * server generation Signed-off-by: Matteo Mortari <[email protected]> * with non regression testing Signed-off-by: Matteo Mortari <[email protected]> * linting Signed-off-by: Matteo Mortari <[email protected]> * do not require artifactType Signed-off-by: Isabella do Amaral <[email protected]> * Update clients/python/tests/test_client.py Signed-off-by: Matteo Mortari <[email protected]> --------- Signed-off-by: Matteo Mortari <[email protected]> Signed-off-by: Isabella do Amaral <[email protected]> Co-authored-by: Matteo Mortari <[email protected]> Co-authored-by: Isabella do Amaral <[email protected]>
kubeflow · Nov 5, 2024 · ca249a0 · ca249a0
1 parent 27c1b05
commit ca249a0
Show file tree

Hide file tree

Showing 25 changed files with 379 additions and 315 deletions.
diff --git a/Makefile b/Makefile
@@ -100,8 +100,7 @@ internal/server/openapi/api_model_registry_service.go: bin/openapi-generator-cli
 .PHONY: gen/openapi
 gen/openapi: bin/openapi-generator-cli openapi/validate pkg/openapi/client.go
 
-pkg/openapi/client.go: bin/openapi-generator-cli api/openapi/model-registry.yaml
-	rm -rf pkg/openapi
+pkg/openapi/client.go: bin/openapi-generator-cli api/openapi/model-registry.yaml clean-pkg-openapi
 	${OPENAPI_GENERATOR} generate \
 		-i api/openapi/model-registry.yaml -g go -o pkg/openapi --package-name openapi \
 		--ignore-file-override ./.openapi-generator-ignore --additional-properties=isGoSubmodule=true,enumClassPrefix=true,useOneOfDiscriminatorLookup=true
@@ -111,9 +110,12 @@ pkg/openapi/client.go: bin/openapi-generator-cli api/openapi/model-registry.yaml
 vet:
 	${GO} vet ./...
 
-.PHONY: clean
+.PHONY: clean-pkg-openapi
+	while IFS= read -r file; do rm -f "pkg/openapi/$file"; done < pkg/openapi/.openapi-generator/FILES
+
+.PHONY: clean clean-pkg-openapi
 clean:
-	rm -Rf ./model-registry internal/ml_metadata/proto/*.go internal/converter/generated/*.go pkg/openapi internal/server/openapi/api_model_registry_service.go
+	rm -Rf ./model-registry internal/ml_metadata/proto/*.go internal/converter/generated/*.go internal/server/openapi/api_model_registry_service.go
 
 .PHONY: clean/odh
 clean/odh:

diff --git a/api/openapi/model-registry.yaml b/api/openapi/model-registry.yaml
@@ -1093,50 +1093,43 @@ components:
       type: string
     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"
         - $ref: "#/components/schemas/DocArtifactCreate"
+        - type: object
+          properties:
+            artifactType:
+              type: string
+              default: "doc-artifact"
     DocArtifactCreate:
       description: A document artifact to be created.
-      type: object
-      required:
-        - artifactType
-      properties:
-        artifactType:
-          type: string
-          default: "doc-artifact"
       allOf:
         - $ref: "#/components/schemas/BaseArtifactCreate"
         - $ref: "#/components/schemas/DocArtifactUpdate"
+        - type: object
+          properties:
+            artifactType:
+              type: string
+              default: "doc-artifact"
     DocArtifactUpdate:
       description: A document artifact to be updated.
-      required:
-        - artifactType
-      properties:
-        artifactType:
-          type: string
-          default: "doc-artifact"
       allOf:
         - $ref: "#/components/schemas/BaseArtifactUpdate"
+        - type: object
+          properties:
+            artifactType:
+              type: string
+              default: "doc-artifact"
     RegisteredModel:
       description: A registered model in model registry. A registered model has ModelVersion children.
       allOf:
@@ -1247,7 +1240,6 @@ components:
         - $ref: "#/components/schemas/BaseExecutionUpdate"
         - $ref: "#/components/schemas/BaseResourceCreate"
     BaseExecutionUpdate:
-      type: object
       allOf:
         - type: object
           properties:
@@ -1438,16 +1430,13 @@ components:
         - $ref: "#/components/schemas/BaseResourceList"
     ModelArtifactUpdate:
       description: An ML model artifact to be updated.
-      required:
-        - artifactType
-      properties:
-        artifactType:
-          type: string
-          default: "model-artifact"
       allOf:
         - $ref: "#/components/schemas/BaseArtifactUpdate"
         - type: object
           properties:
+            artifactType:
+              type: string
+              default: "model-artifact"
             modelFormatName:
               description: Name of the model format.
               type: string
@@ -1465,8 +1454,6 @@ components:
               type: string
     ModelArtifactCreate:
       description: An ML model artifact.
-      required:
-        - artifactType
       properties:
         artifactType:
           type: string
@@ -1562,7 +1549,6 @@ components:
       description: A Model Serving environment for serving `RegisteredModels`.
       allOf:
         - $ref: "#/components/schemas/BaseResource"
-        - type: object
         - $ref: "#/components/schemas/ServingEnvironmentCreate"
     ServingEnvironmentUpdate:
       description: A Model Serving environment for serving `RegisteredModels`.

diff --git a/clients/python/src/mr_openapi/models/doc_artifact.py b/clients/python/src/mr_openapi/models/doc_artifact.py
@@ -25,7 +25,6 @@
 class DocArtifact(BaseModel):
     """A document."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",
@@ -57,6 +56,7 @@ class DocArtifact(BaseModel):
         description="Output only. Last update time of the resource since epoch in millisecond since epoch.",
         alias="lastUpdateTimeSinceEpoch",
     )
+    artifact_type: StrictStr | None = Field(default="doc-artifact", alias="artifactType")
     __properties: ClassVar[list[str]] = [
         "customProperties",
         "description",

diff --git a/clients/python/src/mr_openapi/models/doc_artifact_create.py b/clients/python/src/mr_openapi/models/doc_artifact_create.py
@@ -25,7 +25,6 @@
 class DocArtifactCreate(BaseModel):
     """A document artifact to be created."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",
@@ -46,6 +45,7 @@ class DocArtifactCreate(BaseModel):
         default=None,
         description="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.",
     )
+    artifact_type: StrictStr | None = Field(default="doc-artifact", alias="artifactType")
     __properties: ClassVar[list[str]] = [
         "customProperties",
         "description",

diff --git a/clients/python/src/mr_openapi/models/doc_artifact_update.py b/clients/python/src/mr_openapi/models/doc_artifact_update.py
@@ -25,7 +25,6 @@
 class DocArtifactUpdate(BaseModel):
     """A document artifact to be updated."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",
@@ -42,7 +41,15 @@ class DocArtifactUpdate(BaseModel):
         description="The uniform resource identifier of the physical artifact. May be empty if there is no physical artifact.",
     )
     state: ArtifactState | None = None
-    __properties: ClassVar[list[str]] = ["customProperties", "description", "externalId", "uri", "state"]
+    artifact_type: StrictStr | None = Field(default="doc-artifact", alias="artifactType")
+    __properties: ClassVar[list[str]] = [
+        "customProperties",
+        "description",
+        "externalId",
+        "uri",
+        "state",
+        "artifactType",
+    ]
 
     model_config = ConfigDict(
         populate_by_name=True,
@@ -110,5 +117,6 @@ def from_dict(cls, obj: dict[str, Any] | None) -> Self | None:
                 "externalId": obj.get("externalId"),
                 "uri": obj.get("uri"),
                 "state": obj.get("state"),
+                "artifactType": obj.get("artifactType") if obj.get("artifactType") is not None else "doc-artifact",
             }
         )
diff --git a/clients/python/src/mr_openapi/models/model_artifact.py b/clients/python/src/mr_openapi/models/model_artifact.py
@@ -25,7 +25,6 @@
 class ModelArtifact(BaseModel):
     """An ML model artifact."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",
@@ -57,6 +56,7 @@ class ModelArtifact(BaseModel):
         description="Output only. Last update time of the resource since epoch in millisecond since epoch.",
         alias="lastUpdateTimeSinceEpoch",
     )
+    artifact_type: StrictStr | None = Field(default="model-artifact", alias="artifactType")
     model_format_name: StrictStr | None = Field(
         default=None, description="Name of the model format.", alias="modelFormatName"
     )

diff --git a/clients/python/src/mr_openapi/models/model_artifact_create.py b/clients/python/src/mr_openapi/models/model_artifact_create.py
@@ -25,7 +25,7 @@
 class ModelArtifactCreate(BaseModel):
     """An ML model artifact."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
+    artifact_type: StrictStr | None = Field(default="model-artifact", alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",

diff --git a/clients/python/src/mr_openapi/models/model_artifact_update.py b/clients/python/src/mr_openapi/models/model_artifact_update.py
@@ -25,7 +25,6 @@
 class ModelArtifactUpdate(BaseModel):
     """An ML model artifact to be updated."""  # noqa: E501
 
-    artifact_type: StrictStr = Field(alias="artifactType")
     custom_properties: dict[str, MetadataValue] | None = Field(
         default=None,
         description="User provided custom properties which are not defined by its type.",
@@ -42,6 +41,7 @@ class ModelArtifactUpdate(BaseModel):
         description="The uniform resource identifier of the physical artifact. May be empty if there is no physical artifact.",
     )
     state: ArtifactState | None = None
+    artifact_type: StrictStr | None = Field(default="model-artifact", alias="artifactType")
     model_format_name: StrictStr | None = Field(
         default=None, description="Name of the model format.", alias="modelFormatName"
     )
@@ -61,6 +61,7 @@ class ModelArtifactUpdate(BaseModel):
         "externalId",
         "uri",
         "state",
+        "artifactType",
         "modelFormatName",
         "storageKey",
         "storagePath",
@@ -134,6 +135,7 @@ def from_dict(cls, obj: dict[str, Any] | None) -> Self | None:
                 "externalId": obj.get("externalId"),
                 "uri": obj.get("uri"),
                 "state": obj.get("state"),
+                "artifactType": obj.get("artifactType") if obj.get("artifactType") is not None else "model-artifact",
                 "modelFormatName": obj.get("modelFormatName"),
                 "storageKey": obj.get("storageKey"),
                 "storagePath": obj.get("storagePath"),

diff --git a/clients/python/src/mr_openapi/models/registered_model.py b/clients/python/src/mr_openapi/models/registered_model.py
@@ -37,7 +37,7 @@ class RegisteredModel(BaseModel):
         alias="externalId",
     )
     name: StrictStr = Field(
-        description="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."
+        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."
     )
     id: StrictStr | None = Field(default=None, description="The unique server generated id of the resource.")
     create_time_since_epoch: StrictStr | None = Field(

diff --git a/clients/python/src/mr_openapi/models/registered_model_create.py b/clients/python/src/mr_openapi/models/registered_model_create.py
@@ -37,7 +37,7 @@ class RegisteredModelCreate(BaseModel):
         alias="externalId",
     )
     name: StrictStr = Field(
-        description="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."
+        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."
     )
     owner: StrictStr | None = None
     state: RegisteredModelState | None = None

diff --git a/clients/python/tests/test_client.py b/clients/python/tests/test_client.py
@@ -2,6 +2,7 @@
 from itertools import islice
 
 import pytest
+import requests
 
 from model_registry import ModelRegistry, utils
 from model_registry.exceptions import StoreError
@@ -162,6 +163,39 @@ async def test_update_logical_model_with_labels(client: ModelRegistry):
     assert ma.custom_properties == ma_labels
 
 
+@pytest.mark.e2e
+async def test_patch_model_artifacts_artifact_type(client: ModelRegistry):
+    """Patching ModelArtifact requires `artifactType` value which was previously not required
+
+    reported with https://issues.redhat.com/browse/RHOAIENG-15326
+    """
+    name = "test_model"
+    version = "1.0.0"
+    rm = client.register_model(
+        name,
+        "s3",
+        model_format_name="test_format",
+        model_format_version="test_version",
+        version=version,
+    )
+    assert rm.id
+    mv = client.get_model_version(name, version)
+    assert mv
+    assert mv.id
+    ma = client.get_model_artifact(name, version)
+    assert ma
+    assert ma.id
+
+    payload = { "modelFormatName": "foo" }
+    from .conftest import REGISTRY_HOST, REGISTRY_PORT
+    response = requests.patch(url=f"{REGISTRY_HOST}:{REGISTRY_PORT}/api/model_registry/v1alpha3/model_artifacts/{ma.id}", json=payload, timeout=10, headers={"Content-Type": "application/json"})
+    assert response.status_code == 200
+    ma = client.get_model_artifact(name, version)
+    assert ma
+    assert ma.id
+    assert ma.model_format_name == "foo"
+
+
 @pytest.mark.e2e
 async def test_update_preserves_model_info(client: ModelRegistry):
     name = "test_model"