-
Notifications
You must be signed in to change notification settings - Fork 151
feat(catalog): add generated Python client for Model Catalog API #2065
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
feat(catalog): add generated Python client for Model Catalog API #2065
Conversation
Signed-off-by: Alessio Pragliola <[email protected]>
8229a5b to
9c2b57c
Compare
Signed-off-by: Alessio Pragliola <[email protected]>
Signed-off-by: Alessio Pragliola <[email protected]>
pboyd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think this will be useful.
I was trying to follow the docs but I keep running into errors related to missing tests.
$ make deploy
...
/bin/sh: line 1: cd: ../../../manifests/kustomize/options/catalog/overlays/e2e: No such file or directory
$ make test-e2e
CATALOG_URL=${CATALOG_URL:-http://localhost:8081} poetry run pytest --e2e -v -rA
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --e2e
inifile: /home/pboyd/dev/kubeflow/model-registry/catalog/clients/python/pyproject.toml
rootdir: /home/pboyd/dev/kubeflow/model-registry/catalog/clients/python
make: *** [Makefile:89: test-e2e] Error 4
(make test-fuzz is similar)
Should those be removed from the docs and Makefile for now? Or, maybe, if it's simple enough could you add stubs so those commands work?
|
|
||
| Python client and E2E tests for the Kubeflow Model Catalog. | ||
|
|
||
| ## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this mention virtualenv? I guess it's not absolutely required, but practically most folks will need virtualenv or uv or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Installation | |
| ## Installation | |
| ```bash | |
| # Install dependencies (Poetry creates a virtual environment automatically) | |
| poetry install | |
| # Or, if you prefer to manage your own virtualenv: | |
| python -m venv .venv | |
| source .venv/bin/activate # On Windows: .venv\Scripts\activate | |
| pip install -e . |
something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, like that. Except, they should still run poetry instead of pip, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thanks
|
Nevermind, I didn't see #2066. /lgtm |
Signed-off-by: Alessio Pragliola <[email protected]>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pboyd The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Adds a Python client library for the Model Catalog API.
What's Included
model_catalog.CatalogAPIClient- High-level client with methods for sources, models, artifacts, filtering, and source previewCatalogError,CatalogConnectionError,CatalogAPIError,CatalogNotFoundError,CatalogValidationErrorcatalog_openapi/) - Auto-generated fromapi/openapi/catalog.yamlQuick Example
Follow-up
E2E tests and Kustomize deployment overlay in separate PR
How Has This Been Tested?
make lintMerge criteria:
DCOcheck)ok-to-testhas been added to the PR.