From 467e4fb811b72b0720b23bfd354a379e7e985cd2 Mon Sep 17 00:00:00 2001 From: Isabella Basso Date: Thu, 29 Aug 2024 12:31:04 -0300 Subject: [PATCH] Drop logical model mapping tests (#320) * CONTRIBUTING: Get rid of remote-mlmd recommendations Signed-off-by: Isabella do Amaral * robot: drop logical model mapping tests Since [1] the Python client doesn't access MLMD directly, instead routing requests through the MR server, there's no need to check for matching implementations. [1]: 9b9be508 "Switch Python client backend to use REST (#152)" Signed-off-by: Isabella do Amaral --------- Signed-off-by: Isabella do Amaral --- .github/workflows/run-robot-tests.yaml | 28 ++++------ CONTRIBUTING.md | 11 ---- test/robot/MLMetadata.py | 18 ------ test/robot/MRandLogicalModel.robot | 77 -------------------------- test/robot/requirements.txt | 1 - 5 files changed, 12 insertions(+), 123 deletions(-) delete mode 100644 test/robot/MLMetadata.py delete mode 100644 test/robot/MRandLogicalModel.robot diff --git a/.github/workflows/run-robot-tests.yaml b/.github/workflows/run-robot-tests.yaml index 029b40408..1ffe06142 100644 --- a/.github/workflows/run-robot-tests.yaml +++ b/.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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a98e2bcc7..94bcbc8fb 100644 --- a/CONTRIBUTING.md +++ b/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)). diff --git a/test/robot/MLMetadata.py b/test/robot/MLMetadata.py deleted file mode 100644 index eed40672a..000000000 --- a/test/robot/MLMetadata.py +++ /dev/null @@ -1,18 +0,0 @@ -from ml_metadata import proto -from ml_metadata.metadata_store import metadata_store -from ml_metadata.proto import metadata_store_pb2 - - -class MLMetadata(metadata_store.MetadataStore): - def __init__(self, host: str = "localhost", port: int = 9090): - client_connection_config = metadata_store_pb2.MetadataStoreClientConfig() - client_connection_config.host = host - client_connection_config.port = port - print(client_connection_config) - super().__init__(client_connection_config) - - def get_context_by_single_id(self, context_id: int) -> list[proto.Context]: - return self.get_contexts_by_id([context_id])[0] - - def get_artifact_by_single_id(self, artifact_id: int) -> list[proto.Artifact]: - return self.get_artifacts_by_id([artifact_id])[0] diff --git a/test/robot/MRandLogicalModel.robot b/test/robot/MRandLogicalModel.robot deleted file mode 100644 index fbfd82f34..000000000 --- a/test/robot/MRandLogicalModel.robot +++ /dev/null @@ -1,77 +0,0 @@ -*** Settings *** -Library MLMetadata.py -Library Collections -Resource Setup.resource -Resource MRkeywords.resource -Test Setup Test Setup with dummy data - - -*** Comments *** -You can switch between REST and Python flow by environment variables, -as documented in the keyword implementation - - -*** Test Cases *** -Verify basic logical mapping between MR and MLMD - Comment This test ensures basic logical mapping bewteen MR entities and MLMD entities - ... based on the MR logical mapping: - ... RegisteredModel shall result in a MLMD Context - ... ModelVersion shall result in a MLMD Context and parent Context(of RegisteredModel) - ... ModelArtifact shall result in a MLMD Artifact and Attribution(to the parent Context of ModelVersion) - - ${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 - ${rId} Convert To Integer ${rId} - ${vId} Convert To Integer ${vId} - ${aId} Convert To Integer ${aId} - - # RegisteredModel shall result in a MLMD Context - ${mlmdProto} Get Context By Single Id ${rId} - Log To Console ${mlmdProto} - Should be equal ${mlmdProto.type} kf.RegisteredModel - Should be equal ${mlmdProto.name} ${name} - - # ModelVersion shall result in a MLMD Context and parent Context(of RegisteredModel) - ${mlmdProto} Get Context By Single Id ${vId} - Log To Console ${mlmdProto} - Should be equal ${mlmdProto.type} kf.ModelVersion - Should be equal ${mlmdProto.name} ${rId}:v1 - ${mlmdProto} Get Parent Contexts By Context ${vId} - Should be equal ${mlmdProto[0].id} ${rId} - - # ModelArtifact shall result in a MLMD Artifact and Attribution(to the parent Context of ModelVersion) - ${aNamePrefix} Set Variable ${vId}: - ${mlmdProto} Get Artifact By Single Id ${aId} - Log To Console ${mlmdProto} - Should be equal ${mlmdProto.type} kf.ModelArtifact - Should Start With ${mlmdProto.name} ${aNamePrefix} - Should be equal ${mlmdProto.uri} s3://12345 - ${mlmdProto} Get Artifacts By Context ${vId} - Should be equal ${mlmdProto[0].id} ${aId} - -Verify logical mapping of description property between MR and MLMD - Comment This test ensures logical mapping of the description bewteen MR entities and MLMD entities - ... being implemented as a custom_property - - 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 - ${rId} Given I create a RegisteredModel payload=${registered_model} - ${vId} And I create a child ModelVersion registeredModelID=${rId} payload=&{model_version} - ${aId} And I create a child ModelArtifact modelversionId=${vId} payload=&{model_artifact} - ${rId} Convert To Integer ${rId} - ${vId} Convert To Integer ${vId} - ${aId} Convert To Integer ${aId} - - # RegisteredModel description - ${mlmdProto} Get Context By Single Id ${rId} - Should be equal ${mlmdProto.properties['description'].string_value} Lorem ipsum dolor sit amet - - # ModelVersion description - ${mlmdProto} Get Context By Single Id ${vId} - Should be equal ${mlmdProto.properties['description'].string_value} consectetur adipiscing elit - - # ModelArtifact description - ${mlmdProto} Get Artifact By Single Id ${aId} - Should be equal ${mlmdProto.properties['description'].string_value} sed do eiusmod tempor incididunt diff --git a/test/robot/requirements.txt b/test/robot/requirements.txt index 4b85b513d..d6c4c1b17 100644 --- a/test/robot/requirements.txt +++ b/test/robot/requirements.txt @@ -1,4 +1,3 @@ -ml-metadata==1.14.0 robotframework robotframework-requests pyyaml