[Ongoing] Capture gaps in MR codebase to make more pragmatic
Ongoing, we should capture gaps or issues in the MR codebase (both backend and clients) to make the eventual tackle codebase simpler, easier to work on, developer friendly and maintainable.
Even if they are not "gaps", but are "gotcha's" we should also capture.
To start, suggestion to eventually modularize the client code more and split out larger files. https://github.com/kubeflow/model-registry/pull/1111#issuecomment-2883991681
consider implementing config mgt a-la 12 factor apps with Gila (or analogous). https://github.com/kubeflow/model-registry/pull/1031#discussion_r2066732610
I would consider refactoring the Storage to OCI auth done with:
- https://github.com/kubeflow/model-registry/pull/1031
to make use of temporary files (with secure usage that NamedTemporaryFile extends).
This can be achieved for both Skopeo and Oras backends, for which Push example is provided below:
# example
skopeo_push("download", "quay.io/mmortari/demo20250519:latest", ["--debug", "--dest-authfile", "<omitted>"])
# example
oras_push("download", "quay.io/mmortari/demo20250519:latest", ["--verbose", "--to-registry-config", "<omitted>"])
this would also simplify the handling of the current function wrapping.
@tarilabs yes we can do that. We can address in a cleanup for it at a later date?
thanks @syntaxsdev in my view https://github.com/kubeflow/model-registry/issues/1115#issuecomment-2890292128 is of higher priority
I believe we should move
as_mlops_engineer_i_ itests the Python client tests and move them to a dedicated file to de-clutter the tests.
re: https://github.com/kubeflow/model-registry/blob/a56dc202495c6a04d3ab90243befecf072cf29f3/clients/python/tests/test_client.py#L1181
and re: https://github.com/kubeflow/model-registry/blob/a56dc202495c6a04d3ab90243befecf072cf29f3/clients/python/tests/regression_test.py#L140
please note they are also in different files.
why not combine into one persona test file?
What do you think? @dbasunag @lugi0
I like the idea of dedicated test files based on persona or even features. This would make the tests more organized. While we are at it, I would also suggest 1) renaming tests like test_as_mlops_engineer_i_would_like_to_store_a_malformed_registered_model_i_get_a_structured_error_message to something short. We already have nice descriptions for each tests (we can add for any tests that misses descriptions). 2) parameterize tests when appropriate - that would reduce duplicate test code and encourage more code reusability. I totally understand if that happens over time with multiple PRs.