Add Python client #77
Conversation
|
@isinyaaa I think one of the suggestions from @rimolive from DSP team was that we could re-use mlmd base classes and extend our types from them. |
|
v2 updates:
|
I'm focusing on parity on the base type definitions and on the registry client. The store wrapper should simply provide a useful interface to gRPC focusing on our use case (upserting and querying single protos). |
|
Changes in v3:
|
| @@ -0,0 +1,44 @@ | |||
| [tool.poetry] | |||
There was a problem hiding this comment.
Did you encounter any issues so far by using poetry?
There was a problem hiding this comment.
I had some issues with Python versions, and it's also not as clear how to get proper integration with an editor (I've been using VenvSelector on vim with some hiccups on my Mac).
| @@ -0,0 +1,12 @@ | |||
| """Main package for the ODH model registry.""" | |||
|
|
|||
| __version__ = "0.1.0" | |||
There was a problem hiding this comment.
Also related to the above: I find it rather cumbersome to have version numbers hardcoded in code and/or pyproject.toml.
To solve this issue, I like building with setuptools and using setuptools_scm, which makes it easy to retrieve version information from git.
If you like to stick with poetry, something that might work with poetry is this poetry-dynamic-versioning, although I have not used it before. If you want to do some reading, there's quite a few issues in the poetry repo related to this problem.
There was a problem hiding this comment.
I'd had a look at that and thought it was unnecessary for now, as we are still iterating on an alpha version. This plugin you sent seems to rely on git tags, which we can't use, unless we share tags for the go MR as well as the python client.
| if isinstance(value, bool): | ||
| mlmd_props[key].bool_value = value | ||
| elif isinstance(value, int): | ||
| mlmd_props[key].int_value = value | ||
| elif isinstance(value, float): | ||
| mlmd_props[key].double_value = value | ||
| elif isinstance(value, str): | ||
| mlmd_props[key].string_value = value | ||
| else: | ||
| raise Exception(f"Unsupported type: {type(value)}") |
There was a problem hiding this comment.
Seems like a good place to use structural pattern matching: https://docs.python.org/3/whatsnew/3.10.html?highlight=match#pep-634-structural-pattern-matching
There was a problem hiding this comment.
I had tried this before only to get Irrefutable pattern is allowed only for the last case statement, but now I see I should be matching against an instance and not the type itself.
There was a problem hiding this comment.
Unfortunately it looks like this isn't supported in 3.9... I wouldn't mind dropping compat for this version if we were able to use 3.11, but the MLMD py lib only goes so far as 3.10. Not sure if it's OK to only provide compat for a single version...
There was a problem hiding this comment.
Imho you can consider Dhiraj and Daniele's comments in further PR, if you prefer to merge this now, and could be further improved directly on main for their suggestions and further dev work.
For the records for QE and others;
I've tested this both in DevContainer (I personally find that convenient for IDE support):
and Rosetta emulation:
$env /usr/bin/arch -x86_64 /bin/zsh --login
# then
poetry install
poetry build
poetry run pytest
Create a Python module for the model registry. We'll use gRPC to communicate with a wrapper of the MLMD C++ server. For that, we'll use the MLMD Python module because of its simplified interface, but also define our own as we want to decouple from their implementation in the future. Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Models are artifacts, but when versioned we want to associate them with other artifacts that belong together and compose a deployable unit. As a first implementation, let's not concern with other specific types of artifacts. Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
|
Changes in v4:
|
We want to be able to map those types to the MLMD gRPC types, so let's use a interface to keep the fiddly data manipulation close to the data. Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
This simplifies the direct interface to gRPC provided by MLMD library for the client we want to expose: - we're usually dealing with single instances of proto types - in step with the Go client Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
This sets the ground for our public interface, and should the client's "low-level" interface to the registry. Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Because of a limitation in the MLMD C++ server, names can't be repeated for the same types of artifacts, contexts, etc. so we need to prefix names to be able to allow the users to have more freedom when choosing them. Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella Basso do Amaral <idoamara@redhat.com>
|
Changes in v5:
|
Add a preliminary Python client implementation.
Description
I've based my implementation on #68 and our OpenAPI definition.
How Has This Been Tested?
There are simple tests included. I still have to add instructions to run, a CI job to run them
automatically, and docstrings.
To test, first make sure you have at least Python 3.10 installed, if you don't:
pyenvpyenv install 3.10.13Now, install
nox, then, to test, follow the instructions on the README.md file.TODO:
nox, etc on the READMEMerge criteria:
I don't know if it makes sense to squash this, we can discuss that.