Merge pull request #116 from tarilabs/tarilabs-20240903-sync

periodic sync upstream KF to midstream ODH

.github/workflows/python-tests.yml

@@ -65,6 +65,7 @@ jobs:
         working-directory: clients/python
         run: |
           if [[ ${{ matrix.session }} == "tests" ]]; then
+            make build-mr
             nox --python=${{ matrix.python }} -- --cov-report=xml
           elif [[ ${{ matrix.session }} == "mypy" ]]; then
             nox --python=${{ matrix.python }} ||\

.github/workflows/run-robot-tests.yaml

@@ -6,20 +6,20 @@ on:
   push:
     # To any branch
     branches:
-      - '*'
+      - "*"
   # For every pull request
   pull_request:
     # But ignore this paths
     paths-ignore:
-      - 'LICENSE*'
-      - 'DOCKERFILE*'
-      - '**.gitignore'
-      - '**.md'
-      - '**.txt'
-      - '.github/ISSUE_TEMPLATE/**'
-      - '.github/dependabot.yml'
-      - 'docs/**'
-      - 'scripts/**'
+      - "LICENSE*"
+      - "DOCKERFILE*"
+      - "**.gitignore"
+      - "**.md"
+      - "**.txt"
+      - ".github/ISSUE_TEMPLATE/**"
+      - ".github/dependabot.yml"
+      - "docs/**"
+      - "scripts/**"
 # Define workflow jobs
 jobs:
   # Job runs Robot Framework tests against locally build image from current code
@@ -36,9 +36,9 @@ jobs:
         uses: actions/setup-python@v5
         with:
           # Set Python version to install
-          python-version: '3.9'
+          python-version: "3.9"
           # Set architecture of Python to install
-          architecture: 'x64'
+          architecture: "x64"
       # Install required Python packages for running Robot Framework tests
       - name: Install required Python packages
         # Install required Python packages using pip
@@ -55,10 +55,6 @@ jobs:
       - name: Run Robot Framework tests (REST mode)
         # Run Robot Framework tests in REST mode from test/robot directory
         run: robot test/robot
-      # Run Robot Framework tests in Python mode against running docker compose
-      - name: Run Robot Framework tests (Python mode)
-        # Run Robot Framework tests in Python mode from test/robot directory
-        run: TEST_MODE=Python robot test/robot/MRandLogicalModel.robot
       # Shutdown docker compose with locally build image from current code
       - name: Shutdown docker compose with local image
         # Shutdown docker compose running in the background

.github/workflows/ui-frontend-build.yml

@@ -26,7 +26,7 @@ jobs:
 
       - name: Run tests
         working-directory: clients/ui/frontend
-        run: npm run test:cypress-ci
+        run: npm run test
 
       - name: Clean
         working-directory: clients/ui/frontend

CONTRIBUTING.md

@@ -108,17 +108,6 @@ This colima setups allows to:
 - launch Integration tests in Go (used in Core go layer) with Testcontainers for Go
 - launch DevContainer to be able to install MLMD python wheel dependency (which is x86 specific)
 
-## Remote-only MLMD Python library
-
-The Model Registry Python client wraps `ml-metadata` python dependency (which is [x86 specific](https://pypi.org/project/ml-metadata/#files)).
-
-When developing and contributing to the Model Registry Python client using Apple-silicon/ARM-based computers,
-it can be helpful to evaluate using locally this soft-fork of the upstream MLMD library supporting only remote gRPC connections:
-https://github.com/opendatahub-io/ml-metadata/releases/tag/v1.14.0%2Bremote.1
-
-This dependency can be substituted in the [clients/python/pyproject.toml](clients/python/pyproject.toml) and installed locally with `poetry lock`.
-It is recommended not to check-in this substitution, only helpful for local development while using Apple-silicon/ARM-based computers.
-
 ## DevContainer
 
 Using a [DevContainer](https://containers.dev) is helpful to develop with the Model Registry Python client, since it needs to wrap MLMD python dependency (which is [x86 specific](https://pypi.org/project/ml-metadata/#files)).

clients/python/Makefile

@@ -1,5 +1,8 @@
 all: install tidy
 
+IMG_REGISTRY ?= quay.io
+IMG_VERSION ?= latest 
+
 .PHONY: install
 install:
 	../../bin/openapi-generator-cli generate -i ../../api/openapi/model-registry.yaml -g python -o src/ --package-name mr_openapi --additional-properties=library=asyncio,generateSourceCodeOnly=true,useOneOfDiscriminatorLookup=true
@@ -11,6 +14,14 @@ install:
 clean:
 	rm -rf src/mr_openapi
 
+.PHONY: build-mr
+build-mr:
+	cd ../../ && IMG_REGISTRY=${IMG_REGISTRY} IMG_VERSION=${IMG_VERSION} make image/build
+
+.PHONY: test-e2e
+test-e2e: build-mr
+	poetry run pytest --e2e -s
+
 .PHONY: test
 test:
 	poetry run pytest -s

clients/python/noxfile.py

@@ -21,7 +21,7 @@
 
 
 package = "model_registry"
-python_versions = ["3.12", "3.11","3.10", "3.9"]
+python_versions = ["3.12", "3.11", "3.10", "3.9"]
 nox.needs_version = ">= 2021.6.6"
 nox.options.sessions = (
     "tests",
@@ -63,6 +63,11 @@ def tests(session: Session) -> None:
     try:
         session.run(
             "pytest",
+            *session.posargs,
+        )
+        session.run(
+            "pytest",
+            "--e2e",
             "--cov",
             "--cov-config=pyproject.toml",
             *session.posargs,

clients/python/pyproject.toml

@@ -71,6 +71,7 @@ line-length = 119
 
 [tool.pytest.ini_options]
 asyncio_mode = "auto"
+markers = ["e2e: end-to-end testing"]
 
 [tool.ruff]
 target-version = "py39"

clients/python/tests/basic_test.py → clients/python/tests/REST_bindings_test.py

@@ -43,6 +43,7 @@ async def mv_create(client, rm_create) -> ModelVersionCreate:
     )
 
 
+@pytest.mark.e2e
 async def test_registered_model(client, rm_create):
     rm_create.custom_properties = {
         "key1": MetadataValue.from_dict(
@@ -64,6 +65,7 @@ async def test_registered_model(client, rm_create):
     assert new_rm.description == by_find.description
 
 
+@pytest.mark.e2e
 async def test_model_version(client, mv_create):
     mv_create.custom_properties = {
         "key1": MetadataValue.from_dict(
@@ -87,6 +89,7 @@ async def test_model_version(client, mv_create):
     assert new_mv.custom_properties == by_find.custom_properties
 
 
+@pytest.mark.e2e
 async def test_model_artifact(client, mv_create):
     mv = await client.create_model_version(mv_create)
     assert mv is not None

clients/python/tests/conftest.py

@@ -10,6 +10,24 @@
 import pytest
 import requests
 
+from model_registry import ModelRegistry
+
+
+def pytest_addoption(parser):
+    parser.addoption("--e2e", action="store_true", help="run end-to-end tests")
+
+
+def pytest_collection_modifyitems(config, items):
+    for item in items:
+        skip_e2e = pytest.mark.skip(
+            reason="this is an end-to-end test, requires explicit opt-in --e2e option to run."
+        )
+        if "e2e" in item.keywords:
+            if not config.getoption("--e2e"):
+                item.add_marker(skip_e2e)
+            continue
+
+
 REGISTRY_HOST = "http://localhost"
 REGISTRY_PORT = 8080
 REGISTRY_URL = f"{REGISTRY_HOST}:{REGISTRY_PORT}"
@@ -45,7 +63,7 @@ def poll_for_ready():
         time.sleep(POLL_INTERVAL)
 
 
-@pytest.fixture(scope="session", autouse=True)
+@pytest.fixture(scope="session")
 def _compose_mr(root):
     print("Assuming this is the Model Registry root directory:", root)
     shared_volume = root / "test/config/ml-metadata"
@@ -55,7 +73,7 @@ def _compose_mr(root):
         raise FileExistsError(msg)
     print(f" Starting Docker Compose in folder {root}")
     p = subprocess.Popen(  # noqa: S602
-        f"{DOCKER} compose -f {COMPOSE_FILE} up --build",
+        f"{DOCKER} compose -f {COMPOSE_FILE} up",
         shell=True,
         cwd=root,
     )
@@ -76,7 +94,7 @@ def _compose_mr(root):
 
 
 def cleanup(client):
-    async def yield_and_restart(root):
+    async def yield_and_restart(_compose_mr, root):
         poll_for_ready()
         if inspect.iscoroutinefunction(client) or inspect.isasyncgenfunction(client):
             async with asynccontextmanager(client)() as async_client:
@@ -109,3 +127,9 @@ def event_loop():
     loop = asyncio.get_event_loop_policy().get_event_loop()
     yield loop
     loop.close()
+
+
+@pytest.fixture
+@cleanup
+def client() -> ModelRegistry:
+    return ModelRegistry(REGISTRY_HOST, REGISTRY_PORT, author="author", is_secure=False)

clients/python/tests/regression_test.py

@@ -0,0 +1,24 @@
+import pytest
+
+from model_registry import ModelRegistry
+
+
+@pytest.mark.e2e
+def test_create_tagged_version(client: ModelRegistry):
+    """Test regression for creating tagged versions.
+
+    Reported on: https://github.com/kubeflow/model-registry/issues/255
+    """
+    name = "test_model"
+    version = "model:latest"
+    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

clients/python/tests/test_client.py

@@ -6,14 +6,6 @@
 from model_registry import ModelRegistry, utils
 from model_registry.exceptions import StoreError
 
-from .conftest import REGISTRY_HOST, REGISTRY_PORT, cleanup
-
-
-@pytest.fixture
-@cleanup
-def client() -> ModelRegistry:
-    return ModelRegistry(REGISTRY_HOST, REGISTRY_PORT, author="author", is_secure=False)
-
 
 def test_secure_client():
     os.environ["CERT"] = ""
@@ -24,6 +16,7 @@ def test_secure_client():
     assert "user token" in str(e.value).lower()
 
 
+@pytest.mark.e2e
 async def test_register_new(client: ModelRegistry):
     name = "test_model"
     version = "1.0.0"
@@ -44,6 +37,7 @@ async def test_register_new(client: ModelRegistry):
     assert ma
 
 
+@pytest.mark.e2e
 async def test_register_new_using_s3_uri_builder(client: ModelRegistry):
     name = "test_model"
     version = "1.0.0"
@@ -68,6 +62,7 @@ async def test_register_new_using_s3_uri_builder(client: ModelRegistry):
     assert ma.uri == uri
 
 
+@pytest.mark.e2e
 def test_register_existing_version(client: ModelRegistry):
     params = {
         "name": "test_model",
@@ -82,6 +77,7 @@ def test_register_existing_version(client: ModelRegistry):
         client.register_model(**params)
 
 
+@pytest.mark.e2e
 async def test_get(client: ModelRegistry):
     name = "test_model"
     version = "1.0.0"
@@ -112,6 +108,7 @@ async def test_get(client: ModelRegistry):
     assert ma.id == _ma.id
 
 
+@pytest.mark.e2e
 def test_get_registered_models(client: ModelRegistry):
     models = 21
 
@@ -142,6 +139,7 @@ def test_get_registered_models(client: ModelRegistry):
     assert i == models
 
 
+@pytest.mark.e2e
 def test_get_registered_models_and_reset(client: ModelRegistry):
     model_count = 6
     page = model_count // 2
@@ -166,6 +164,7 @@ def test_get_registered_models_and_reset(client: ModelRegistry):
     assert complete[:page] == models
 
 
+@pytest.mark.e2e
 def test_get_model_versions(client: ModelRegistry):
     name = "test_model"
     models = 21
@@ -197,6 +196,7 @@ def test_get_model_versions(client: ModelRegistry):
     assert i == models
 
 
+@pytest.mark.e2e
 def test_get_model_versions_and_reset(client: ModelRegistry):
     name = "test_model"
 
@@ -223,6 +223,7 @@ def test_get_model_versions_and_reset(client: ModelRegistry):
     assert complete[:page] == models
 
 
+@pytest.mark.e2e
 def test_hf_import(client: ModelRegistry):
     pytest.importorskip("huggingface_hub")
     name = "openai-community/gpt2"
@@ -250,6 +251,7 @@ def test_hf_import(client: ModelRegistry):
     assert client.get_model_artifact(name, version)
 
 
+@pytest.mark.e2e
 def test_hf_import_default_env(client: ModelRegistry):
     """Test setting environment variables, hence triggering defaults, does _not_ interfere with HF metadata"""
     pytest.importorskip("huggingface_hub")