Skip to content
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

Context name containing characters requiring url quoting/escaping #255

Closed
tarilabs opened this issue Aug 11, 2024 · 1 comment · Fixed by #295
Closed

Context name containing characters requiring url quoting/escaping #255

tarilabs opened this issue Aug 11, 2024 · 1 comment · Fixed by #295
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@tarilabs
Copy link
Member

Describe the bug
Unable to index ModelVersion containing :.

To Reproduce
Steps to reproduce the behavior:

  1. attempt to index a ModelVersion for "some:version"

following the kubeflow tutorial, example:
,

rm = registry.register_model(
    "mnist",
    "https://github.com/tarilabs/demo20231212/raw/main/v1.nb20231206162408/mnist.onnx",
    model_format_name="onnx",
    model_format_version="1",
    version="some:version",    # <-- here
    description="lorem ipsum mnist",
    metadata={
        "accuracy": 3.14,
        "license": "apache-2.0",
    }
)

Expected behavior
Can use special characters without having to urllib.parse.quote() manually the version name.

i.e. without requiring to:

from urllib.parse import quote
# ...
    version=quote("some:version"),   

Additional context
This is likely deriving from the fact that in the Model Registry Python client, the logical model entity attributes are passed to the rest layer without quote where sometimes might be necessary; for example, when the model version name is used as a query parameter in a rest endpoint call.

It could be a helpful exercise to check if any limitations on MLMD side for Context names themselves (we are aware MLMD Context names are not updatable, but that is an unrelated limitation).

@tarilabs tarilabs added bug Something isn't working good first issue Good for newcomers labels Aug 11, 2024
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Aug 22, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Aug 26, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Aug 27, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
dhirajsb pushed a commit to dhirajsb/model-registry-kfp that referenced this issue Aug 30, 2024
…eflow#255)

Bumps [github.com/go-chi/chi/v5](https://github.com/go-chi/chi) from 5.0.10 to 5.0.11.
- [Release notes](https://github.com/go-chi/chi/releases)
- [Changelog](https://github.com/go-chi/chi/blob/master/CHANGELOG.md)
- [Commits](go-chi/chi@v5.0.10...v5.0.11)

---
updated-dependencies:
- dependency-name: github.com/go-chi/chi/v5
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Aug 30, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Aug 30, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Sep 2, 2024
Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
@tarilabs
Copy link
Member Author

tarilabs commented Sep 2, 2024

thank you @isinyaaa

Al-Pragliola pushed a commit to Al-Pragliola/model-registry that referenced this issue Sep 3, 2024
* converter: simplify MapName

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>

* converter: restrict captures to prefix when mapping *FromOwned

Fixes: kubeflow#255

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>

* py: make: fix build-mr tagging

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>

* py: tests: introduce regression testing

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>

* py: make: expose IMG_* env vars

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>

---------

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
@tarilabs tarilabs added this to the Kubeflow 1.10 roadmap milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant