Merge sync remote-tracking branch 'upstream/main' into tarilabs-20241…

…108-sync

Signed-off-by: Matteo Mortari <[email protected]>

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:

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`.

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",

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",

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",
             }
         )

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"
     )

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.",

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"),

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(

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

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"

clients/ui/bff/Makefile

@@ -3,6 +3,8 @@ IMG ?= model-registry-bff:latest
 PORT ?= 4000
 MOCK_K8S_CLIENT ?= false
 MOCK_MR_CLIENT ?= false
+# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
+ENVTEST_K8S_VERSION = 1.29.0
 
 .PHONY: all
 all: build
@@ -32,15 +34,17 @@ vet:  .
 	go vet ./...
 
 .PHONY: test
-test:
+test: fmt vet envtest
+	ENVTEST_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \
 	go test ./...
 
 .PHONY: build
 build: fmt vet test
 	go build -o bin/bff cmd/main.go
 
 .PHONY: run
-run: fmt vet
+run: fmt vet envtest
+	ENVTEST_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \
 	go run ./cmd/main.go  --port=$(PORT) --mock-k8s-client=$(MOCK_K8S_CLIENT) --mock-mr-client=$(MOCK_MR_CLIENT)
 
 .PHONY: docker-build
@@ -54,8 +58,18 @@ LOCALBIN ?= $(shell pwd)/bin
 $(LOCALBIN):
 	mkdir -p $(LOCALBIN)
 
+## Tool Binaries
+ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION)
 GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
+
+## Tool Versions
 GOLANGCI_LINT_VERSION ?= v1.57.2
+ENVTEST_VERSION ?= release-0.17
+
+.PHONY: envtest
+envtest: $(ENVTEST) ## Download setup-envtest locally if necessary.
+$(ENVTEST): $(LOCALBIN)
+	$(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest,$(ENVTEST_VERSION))
 
 .PHONY: golangci-lint
 golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
@@ -75,4 +89,4 @@ echo "Downloading $${package}" ;\
 GOBIN=$(LOCALBIN) go install $${package} ;\
 mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\
 }
-endef
+endef