Skip to content

Commit

Permalink
Merge pull request #32 from tarilabs/tarilabs-20240326-sync
Browse files Browse the repository at this point in the history
periodic sync upstream KF to midstream ODH
  • Loading branch information
openshift-merge-bot[bot] committed Mar 26, 2024
2 parents bb0a432 + 0ebf88d commit dd093b7
Show file tree
Hide file tree
Showing 18 changed files with 461 additions and 98 deletions.
16 changes: 8 additions & 8 deletions .github/workflows/build-and-push-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ on:
- '.github/dependabot.yml'
- 'docs/**'
env:
QUAY_ORG: opendatahub
QUAY_IMG_REPO: model-registry
QUAY_USERNAME: ${{ secrets.QUAY_USERNAME }}
QUAY_PASSWORD: ${{ secrets.QUAY_PASSWORD }}
IMG_ORG: opendatahub
IMG_REPO: model-registry
DOCKER_USER: ${{ secrets.QUAY_USERNAME }}
DOCKER_PWD: ${{ secrets.QUAY_PASSWORD }}
PUSH_IMAGE: true
jobs:
build-image:
Expand Down Expand Up @@ -50,8 +50,8 @@ jobs:
if: env.BUILD_CONTEXT == 'main'
shell: bash
env:
IMG: quay.io/${{ env.QUAY_ORG }}/${{ env.QUAY_IMG_REPO }}
BUILD_IMAGE: false
IMG: quay.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}
BUILD_IMAGE: false # image is already built in "Build and Push Image" step
run: |
docker tag ${{ env.IMG }}:$VERSION ${{ env.IMG }}:latest
# BUILD_IMAGE=false skip the build, just push the tag made above
Expand All @@ -60,8 +60,8 @@ jobs:
if: env.BUILD_CONTEXT == 'main'
shell: bash
env:
IMG: quay.io/${{ env.QUAY_ORG }}/${{ env.QUAY_IMG_REPO }}
BUILD_IMAGE: false
IMG: quay.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}
BUILD_IMAGE: false # image is already built in "Build and Push Image" step
run: |
docker tag ${{ env.IMG }}:$VERSION ${{ env.IMG }}:main
# BUILD_IMAGE=false skip the build, just push the tag made above
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/build-image-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ on:
- 'docs/**'
- 'clients/python/**'
env:
QUAY_IMG_REPO: model-registry
IMG_ORG: opendatahub
IMG_REPO: model-registry
PUSH_IMAGE: false
BRANCH: ${{ github.base_ref }}
jobs:
Expand All @@ -35,12 +36,12 @@ jobs:
uses: helm/kind-action@v1.9.0
- name: Load Local Registry Test Image
env:
IMG: "quay.io/opendatahub/model-registry:${{ steps.tags.outputs.tag }}"
IMG: "quay.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}"
run: |
kind load docker-image -n chart-testing ${IMG}
- name: Deploy Operator With Test Image
env:
IMG: "quay.io/opendatahub/model-registry:${{ steps.tags.outputs.tag }}"
IMG: "quay.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}"
run: |
echo "Deploying operator from model-registry-operator branch ${BRANCH}"
kubectl apply -k "https://github.com/opendatahub-io/model-registry-operator.git/config/default?ref=${BRANCH}"
Expand Down
15 changes: 12 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ MLMD_VERSION ?= 1.14.0
DOCKER ?= docker
# default Dockerfile
DOCKERFILE ?= Dockerfile
# container registry
# container registry, default to empty (dockerhub) if not explicitly set
IMG_REGISTRY ?= quay.io
# container image organization
IMG_ORG ?= opendatahub
Expand All @@ -21,7 +21,11 @@ IMG_VERSION ?= main
# container image repository
IMG_REPO ?= model-registry
# container image
IMG ?= ${IMG_REGISTRY}/$(IMG_ORG)/$(IMG_REPO)
ifdef IMG_REGISTRY
IMG := ${IMG_REGISTRY}/${IMG_ORG}/${IMG_REPO}
else
IMG := ${IMG_ORG}/${IMG_REPO}
endif

model-registry: build

Expand Down Expand Up @@ -190,7 +194,12 @@ proxy: build
# login to docker
.PHONY: docker/login
docker/login:
$(DOCKER) login -u "${DOCKER_USER}" -p "${DOCKER_PWD}" "${IMG_REGISTRY}"
ifdef IMG_REGISTRY
$(DOCKER) login -u "${DOCKER_USER}" -p "${DOCKER_PWD}" "${IMG_REGISTRY}"
else
$(DOCKER) login -u "${DOCKER_USER}" -p "${DOCKER_PWD}"
endif


# build docker image
.PHONY: image/build
Expand Down
33 changes: 28 additions & 5 deletions clients/python/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ registry = ModelRegistry(server_address="server-address", port=9090, author="aut

model = registry.register_model(
"my-model", # model name
"s3://path/to/model", # model URI
"https://storage-place.my-company.com", # model URI
version="2.0.0",
description="lorem ipsum",
model_format_name="onnx",
model_format_version="1",
storage_key="aws-connection-path",
storage_key="my-data-connection",
storage_path="path/to/model",
metadata={
# can be one of the following types
Expand All @@ -37,10 +37,33 @@ version = registry.get_model_version("my-model", "v2.0")
experiment = registry.get_model_artifact("my-model", "v2.0")
```

### Default values for metadata
### Importing from S3

If not supplied, `metadata` values defaults to a predefined set of conventional values.
Reference the technical documentation in the pydoc of the client.
When registering models stored on S3-compatible object storage, you should use `utils.s3_uri_from` to build an
unambiguous URI for your artifact.

```py
from model_registry import ModelRegistry, utils

registry = ModelRegistry(server_address="server-address", port=9090, author="author")

model = registry.register_model(
"my-model", # model name
uri=utils.s3_uri_from("path/to/model", "my-bucket"),
version="2.0.0",
description="lorem ipsum",
model_format_name="onnx",
model_format_version="1",
storage_key="my-data-connection",
metadata={
# can be one of the following types
"int_key": 1,
"bool_key": False,
"float_key": 3.14,
"str_key": "str_value",
}
)
```

### Importing from Hugging Face Hub

Expand Down
30 changes: 11 additions & 19 deletions clients/python/src/model_registry/_client.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Standard client for the model registry."""

from __future__ import annotations

import os
from typing import get_args
from warnings import warn

Expand Down Expand Up @@ -75,16 +75,22 @@ def register_model(
model_format_name: str,
model_format_version: str,
version: str,
author: str | None = None,
description: str | None = None,
storage_key: str | None = None,
storage_path: str | None = None,
service_account_name: str | None = None,
author: str | None = None,
description: str | None = None,
metadata: dict[str, ScalarType] | None = None,
) -> RegisteredModel:
"""Register a model.
Either `storage_key` and `storage_path`, or `service_account_name` must be provided.
This registers a model in the model registry. The model is not downloaded, and has to be stored prior to
registration.
Most models can be registered using their URI, along with optional connection-specific parameters, `storage_key`
and `storage_path` or, simply a `service_account_name`.
URI builder utilities are recommended when referring to specialized storage; for example `utils.s3_uri_from`
helper when using S3 object storage data connections.
Args:
name: Name of the model.
Expand All @@ -110,7 +116,7 @@ def register_model(
version,
author or self._author,
description=description,
metadata=metadata or self.default_metadata(),
metadata=metadata or {},
)
self._register_model_artifact(
mv,
Expand All @@ -124,19 +130,6 @@ def register_model(

return rm

def default_metadata(self) -> dict[str, ScalarType]:
"""Default metadata valorisations.
When not explicitly supplied by the end users, these valorisations will be used
by default.
Returns:
default metadata valorisations.
"""
return {
key: os.environ[key] for key in ["AWS_S3_ENDPOINT", "AWS_S3_BUCKET", "AWS_DEFAULT_REGION"] if key in os.environ
}

def register_hf_model(
self,
repo: str,
Expand Down Expand Up @@ -202,7 +195,6 @@ def register_hf_model(
model_author = author
source_uri = hf_hub_url(repo, path, revision=git_ref)
metadata = {
**self.default_metadata(),
"repo": repo,
"source_uri": source_uri,
"model_origin": "huggingface_hub",
Expand Down
109 changes: 109 additions & 0 deletions clients/python/src/model_registry/_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
from __future__ import annotations

import functools
import inspect
from collections.abc import Sequence
from typing import Any, Callable, TypeVar

CallableT = TypeVar("CallableT", bound=Callable[..., Any])


# copied from https://github.com/Rapptz/RoboDanny
def human_join(seq: Sequence[str], *, delim: str = ", ", final: str = "or") -> str:
size = len(seq)
if size == 0:
return ""

if size == 1:
return seq[0]

if size == 2:
return f"{seq[0]} {final} {seq[1]}"

return delim.join(seq[:-1]) + f" {final} {seq[-1]}"


def quote(string: str) -> str:
"""Add single quotation marks around the given string. Does *not* do any escaping."""
return f"'{string}'"


# copied from https://github.com/openai/openai-python
def required_args(*variants: Sequence[str]) -> Callable[[CallableT], CallableT]: # noqa: C901
"""Decorator to enforce a given set of arguments or variants of arguments are passed to the decorated function.
Useful for enforcing runtime validation of overloaded functions.
Example usage:
```py
@overload
def foo(*, a: str) -> str:
...
@overload
def foo(*, b: bool) -> str:
...
# This enforces the same constraints that a static type checker would
# i.e. that either a or b must be passed to the function
@required_args(["a"], ["b"])
def foo(*, a: str | None = None, b: bool | None = None) -> str:
...
```
"""

def inner(func: CallableT) -> CallableT: # noqa: C901
params = inspect.signature(func).parameters
positional = [
name
for name, param in params.items()
if param.kind
in {
param.POSITIONAL_ONLY,
param.POSITIONAL_OR_KEYWORD,
}
]

@functools.wraps(func)
def wrapper(*args: object, **kwargs: object) -> object:
given_params: set[str] = set()
for i, _ in enumerate(args):
try:
given_params.add(positional[i])
except IndexError:
msg = f"{func.__name__}() takes {len(positional)} argument(s) but {len(args)} were given"
raise TypeError(msg) from None

for key in kwargs:
given_params.add(key)

for variant in variants:
matches = all(param in given_params for param in variant)
if matches:
break
else: # no break
if len(variants) > 1:
variations = human_join(
[
"("
+ human_join([quote(arg) for arg in variant], final="and")
+ ")"
for variant in variants
]
)
msg = f"Missing required arguments; Expected either {variations} arguments to be given"
else:
# TODO: this error message is not deterministic
missing = list(set(variants[0]) - given_params)
if len(missing) > 1:
msg = f"Missing required arguments: {human_join([quote(arg) for arg in missing])}"
else:
msg = f"Missing required argument: {quote(missing[0])}"
raise TypeError(msg)
return func(*args, **kwargs)

return wrapper # type: ignore

return inner
4 changes: 4 additions & 0 deletions clients/python/src/model_registry/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ class StoreException(Exception):
"""Storage related error."""


class MissingMetadata(Exception):
"""Not enough metadata to complete operation."""


class UnsupportedTypeException(StoreException):
"""Raised when an unsupported type is encountered."""

Expand Down
Loading

0 comments on commit dd093b7

Please sign in to comment.