feat: migrate remainder of User Story from Robot to pytest (and DocArtifact) (kubeflow#1331)

* feat: migrate remainder of User Story from Robot to pytest (and DocArtifact)

- Migrate Robot Framework user stories to pytest with comprehensive test coverage:
  - Model name storage and validation
  - Model description storage and updates
  - Longer documentation storage via DocArtifact
- Enhance ModelVersion with registered_model_id field and improved documentation
- Add DocArtifact create() and update() methods with proper artifact type mapping
- Improve Makefile deployment with conditional image building (helpful locally)
- Add migration comments to existing Robot Framework tests per convention

This change completes the DocArtifact implementation and modernizes the test suite
by migrating user stories from Robot Framework to pytest for better maintainability
and as previously agreed and enacted.

With this PR, all Robot UserStor-ies are migrated to pytest

Signed-off-by: Matteo Mortari <matteo.mortari@gmail.com>

* linting

Signed-off-by: Matteo Mortari <matteo.mortari@gmail.com>

* implement code review feedback

Signed-off-by: Matteo Mortari <matteo.mortari@gmail.com>

---------

Signed-off-by: Matteo Mortari <matteo.mortari@gmail.com>

Author: tarilabs

clients/python/Makefile

@@ -2,6 +2,7 @@ all: install tidy
 
 IMG_VERSION ?= latest
 IMG ?= ghcr.io/kubeflow/model-registry/server:${IMG_VERSION}
+BUILD_IMAGE ?= true # whether to build the MR server image
 
 .PHONY: install
 install:
@@ -16,7 +17,11 @@ clean:
 
 .PHONY: deploy-latest-mr
 deploy-latest-mr:
-	cd ../../ && IMG_VERSION=${IMG_VERSION} IMG=${IMG} make image/build ARGS="--load$(if ${DEV_BUILD}, --target dev-build)"  && LOCAL=1 ./scripts/deploy_on_kind.sh
+	cd ../../ && \
+	$(if $(filter true,$(BUILD_IMAGE)),\
+		IMG_VERSION=${IMG_VERSION} IMG=${IMG} make image/build ARGS="--load$(if ${DEV_BUILD}, --target dev-build)" && \
+	) \
+	LOCAL=1 ./scripts/deploy_on_kind.sh
 	kubectl port-forward -n kubeflow services/model-registry-service 8080:8080 & echo $$! >> .port-forwards.pid
 
 .PHONY: deploy-test-minio

clients/python/src/model_registry/types/artifacts.py

@@ -22,6 +22,8 @@
 )
 from mr_openapi import (
     ArtifactState,
+    DocArtifactCreate,
+    DocArtifactUpdate,
     ModelArtifactCreate,
     ModelArtifactUpdate,
 )
@@ -87,11 +89,23 @@ class DocArtifact(Artifact):
 
     @override
     def create(self, **kwargs) -> Any:
-        raise NotImplementedError
+        """Create a new DocArtifactCreate object."""
+        return DocArtifactCreate(
+            customProperties=self._map_custom_properties(),
+            **self._props_as_dict(exclude=("id", "custom_properties")),
+            artifactType="doc-artifact",
+            **kwargs,
+        )
 
     @override
     def update(self, **kwargs) -> Any:
-        raise NotImplementedError
+        """Create a new DocArtifactUpdate object."""
+        return DocArtifactUpdate(
+            customProperties=self._map_custom_properties(),
+            **self._props_as_dict(exclude=("id", "name", "custom_properties")),
+            artifactType="doc-artifact",
+            **kwargs,
+        )
 
     @override
     def as_basemodel(self) -> DocArtifactBaseModel:

clients/python/src/model_registry/types/contexts.py

@@ -36,15 +36,17 @@ class ModelVersion(BaseResourceModel):
 
     Attributes:
         name: Name of this version.
-        author: Author of the model version.
-        description: Description of the object.
+        author: Author of this model version.
+        state: Status of this model version.
+        description: Description of this object.
         external_id: Customizable ID. Has to be unique among instances of the same type.
         artifacts: Artifacts associated with this version.
     """
 
     name: str
     author: str | None = None
     state: ModelVersionState = ModelVersionState.LIVE
+    registered_model_id: str | None = None
 
     @override
     def create(self, *, registered_model_id: str, **kwargs) -> ModelVersionCreate:  # type: ignore[override]
@@ -75,6 +77,7 @@ def from_basemodel(cls, source: ModelVersionBaseModel) -> ModelVersion:
             author=source.author,
             description=source.description,
             external_id=source.external_id,
+            registered_model_id=source.registered_model_id,
             create_time_since_epoch=source.create_time_since_epoch,
             last_update_time_since_epoch=source.last_update_time_since_epoch,
             custom_properties=cls._unmap_custom_properties(source.custom_properties)

clients/python/tests/test_client.py

@@ -7,6 +7,7 @@
 from model_registry import ModelRegistry, utils
 from model_registry.exceptions import StoreError
 from model_registry.types import ModelArtifact
+from model_registry.types.artifacts import DocArtifact
 
 from .extras.async_task_runner import AsyncTaskRunner
 
@@ -22,23 +23,30 @@ def test_secure_client():
 
 @pytest.mark.e2e
 async def test_register_new(client: ModelRegistry):
+    """As a MLOps engineer I would like to store Model name
+    """
     name = "test_model"
     version = "1.0.0"
     rm = client.register_model(
         name,
-        "s3",
+        "https://acme.org/something",
         model_format_name="test_format",
         model_format_version="test_version",
         version=version,
     )
     assert rm.id
+    assert rm.name == name # check the Model name
 
     mr_api = client._api
     mv = await mr_api.get_model_version_by_params(rm.id, version)
     assert mv
     assert mv.id
+    assert mv.name == version
+    assert mv.registered_model_id == rm.id
+
     ma = await mr_api.get_model_artifact_by_params(name, mv.id)
     assert ma
+    assert ma.uri == "https://acme.org/something"
 
 
 @pytest.mark.e2e
@@ -1086,3 +1094,85 @@ def test_upload_large_model_file(
     mv = client.get_model_artifact(name="large_test_model", version=version)
     assert mv
     assert mv.name == "large_test_model"
+
+
+@pytest.mark.e2e
+async def test_as_mlops_engineer_i_would_like_to_update_a_description_of_the_model(client: ModelRegistry):
+    """As a MLOps engineer I would like to update a description of the model
+    """
+    name = "test_model"
+    version = "1.0.0"
+    rm = client.register_model(
+        name,
+        "https://acme.org/something",
+        model_format_name="test_format",
+        model_format_version="test_version",
+        version=version,
+        owner="me",
+        description="Lorem ipsum dolor sit amet",
+    )
+    assert rm.id
+
+    rm.description = "New description"
+    rm =client.update(rm)
+    assert rm.description == "New description"
+    assert rm.owner == "me"
+
+
+@pytest.mark.e2e
+async def test_as_mlops_engineer_i_would_like_to_store_a_description_of_the_model(client: ModelRegistry):
+    """As a MLOps engineer I would like to store a description of the model
+    Note: on Creation, the Description belongs to the Model Version; we could improve the logic to maintain it for the Registered Model if it's not already existing
+    """
+    name = "test_model"
+    version = "1.0.0"
+    rm = client.register_model(
+        name,
+        "https://acme.org/something",
+        model_format_name="test_format",
+        model_format_version="test_version",
+        version=version,
+        description="consectetur adipiscing elit",
+    )
+    assert rm.id
+
+    mr_api = client._api
+    mv = await mr_api.get_model_version_by_params(rm.id, version)
+    assert mv
+    assert mv.id
+    assert mv.description == "consectetur adipiscing elit"
+    ma = await mr_api.get_model_artifact_by_params(name, mv.id)
+    assert ma
+
+    rm.description = "Lorem ipsum dolor sit amet"
+    assert client.update(rm).description == "Lorem ipsum dolor sit amet"
+    mv.description = "consectetur adipiscing elit2"
+    assert client.update(mv).description == "consectetur adipiscing elit2"
+    ma.description = "sed do eiusmod tempor incididunt"
+    assert client.update(ma).description == "sed do eiusmod tempor incididunt"
+
+
+@pytest.mark.e2e
+async def test_as_mlops_engineer_i_would_like_to_store_a_longer_documentation_for_the_model(client: ModelRegistry):
+    """As a MLOps engineer I would like to store a longer documentation for the model
+    """
+    name = "test_model"
+    version = "1.0.0"
+    rm = client.register_model(
+        name,
+        "https://acme.org/something",
+        model_format_name="test_format",
+        model_format_version="test_version",
+        version=version,
+        description="consectetur adipiscing elit",
+    )
+    assert rm.id
+
+    mr_api = client._api
+    mv = await mr_api.get_model_version_by_params(rm.id, version)
+    assert mv
+    assert mv.id
+
+    da = await mr_api.upsert_model_version_artifact(DocArtifact(uri="https://README.md"), mv.id)
+    assert da
+    assert da.uri == "https://README.md"

test/robot/UserStory.robot

@@ -10,6 +10,7 @@ These User Story(-ies) are defined in the PM document
 
 *** Test Cases ***
 As a MLOps engineer I would like to store Model name
+    # MIGRATED to test_register_new in pytest
     ${rId}  Given I create a RegisteredModel having    name=${name}
     ${vId}  And I create a child ModelVersion having    registeredModelID=${rId}    name=v1
     ${aId}  And I create a child ModelArtifact having    modelversionId=${vId}  uri=s3://12345
@@ -22,6 +23,7 @@ As a MLOps engineer I would like to store Model name
           And Should be equal    ${r["uri"]}    s3://12345
 
 As a MLOps engineer I would like to store a description of the model
+    # MIGRATED to test_as_mlops_engineer_i_would_like_to_store_a_description_of_the_model in pytest
     Set To Dictionary    ${registered_model}    description=Lorem ipsum dolor sit amet  name=${name}
     Set To Dictionary    ${model_version}    description=consectetur adipiscing elit
     Set To Dictionary    ${model_artifact}    description=sed do eiusmod tempor incididunt
@@ -42,6 +44,7 @@ As a MLOps engineer I would like to store a description of the model
           And Should be equal    ${r["description"]}    sed do eiusmod tempor incididunt
 
 As a MLOps engineer I would like to update a description of the model
+    # MIGRATED to test_as_mlops_engineer_i_would_like_to_update_a_description_of_the_model in pytest
     Set To Dictionary    ${registered_model}    description=Lorem ipsum dolor sit amet  name=${name}  owner=me
     ${rId}  Given I create a RegisteredModel    payload=${registered_model}
     &{registered_model_update}  Create dictionary    description=New description
@@ -51,6 +54,7 @@ As a MLOps engineer I would like to update a description of the model
           And Should be equal    ${r["owner"]}    me
 
 As a MLOps engineer I would like to store a longer documentation for the model
+    # MIGRATED to test_as_mlops_engineer_i_would_like_to_store_a_longer_documentation_for_the_model in pytest
     Set To Dictionary    ${registered_model}    description=Lorem ipsum dolor sit amet  name=${name}
     Set To Dictionary    ${model_version}    description=consectetur adipiscing elit
     Set To Dictionary    ${model_artifact}    description=sed do eiusmod tempor incididunt