Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] main from kubeflow:main #115

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/python-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }} ||\
Expand Down
28 changes: 12 additions & 16 deletions .github/workflows/run-robot-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ui-frontend-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
11 changes: 11 additions & 0 deletions clients/python/Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
all: install tidy

IMG_REGISTRY ?= docker.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
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion clients/python/noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 19 additions & 19 deletions clients/python/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions clients/python/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ line-length = 119

[tool.pytest.ini_options]
asyncio_mode = "auto"
markers = ["e2e: end-to-end testing"]

[tool.ruff]
target-version = "py39"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand Down
30 changes: 27 additions & 3 deletions clients/python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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"
Expand All @@ -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,
)
Expand All @@ -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:
Expand Down Expand Up @@ -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)
24 changes: 24 additions & 0 deletions clients/python/tests/regression_test.py
Original file line number Diff line number Diff line change
@@ -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
18 changes: 10 additions & 8 deletions clients/python/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"] = ""
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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",
Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"

Expand All @@ -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"
Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading