Model labels in the openapi models

[lampajr]

Is your feature request related to a problem? Please describe.
As discussed in UI/UX meetings other than key-value pairs (i.e., properties) we also have labels.
Right now openapi models do not have explicit labels field, we should consider adding it.

Describe the solution you'd like
In the openapi spec: add a new field named labels that will contain all labels associated to an entity.

• type: array of strings?
• affected entities:
  • RegisteredModel
  • ModelVersion
  • others?

How can we map these labels in the underlying ml-metadata model?
Create a property named labels of type struct value that can contain the array of strings

Describe alternatives you've considered
n/a

Additional context
n/a

[dhirajsb]

IMHO, we need to avoid having fields/properties called labels. Right now, the API is the service backend, and the concept/term labels is how the UI is representing some custom properties. The case against having labels is made by the fact that the UI won't treat the same field in other resources as labels.

If we go down this path, we start turning the REST API into the BFF pattern I mentioned before. I strongly recommend that we should keep the API closer to the backend and avoid having to change it every time some presentation change is needed in the UI. The UI has a proxy of their own, and they can accommodate these BFF patterns in that layer.

[lampajr]

IMHO, we need to avoid having fields/properties called labels. Right now, the API is the service backend, and the concept/term labels is how the UI is representing some custom properties. The case against having labels is made by the fact that the UI won't treat the same field in other resources as labels.

So are you suggesting to keep the models as they are (i.e., without any explicit field labels) and let UI do this mapping by providing those labels as generic custom properties? Am i right?

[tarilabs]

I'd recommend giving UI folks a pragmatic example, something like:

curl -X 'POST' \
  'http://localhost:8080/api/model_registry/v1alpha1/registered_models' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -d '{
  "customProperties": {
    "labels": {
      "struct_value": "{\"labels\": [\"ocr\",\"img2text\",\"mnist\"]}"
    }
  },
  "description": "handwriting recognition of digits",
  "name": "mnist"
}'

Which should work already, but worth to test it out e2e on our side first.

This way, they can develop to "accommodate these BFF patterns in that layer" as in the conversion function to/from the customProperty in the UI proxy layer.

[tarilabs]

taking base64 encoding of: {"labels": ["ocr", "img2txt", "mnist"]}
is: eyJsYWJlbHMiOiBbIm9jciIsICJpbWcydHh0IiwgIm1uaXN0Il19

which works as:

curl -X 'POST' \
  'http://localhost:8080/api/model_registry/v1alpha3/registered_models' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -d '{
  "customProperties": {
    "labels": {
      "struct_value": "eyJsYWJlbHMiOiBbIm9jciIsICJpbWcydHh0IiwgIm1uaXN0Il19",
      "metadataType":"MetadataStructValue"
    }
  },
  "description": "handwriting recognition of digits",
  "name": "mnist"
}'
{"createTimeSinceEpoch":"1712076638394","customProperties":{"labels":{"metadataType":"MetadataStructValue","struct_value":"eyJsYWJlbHMiOlsib2NyIiwiaW1nMnR4dCIsIm1uaXN0Il19"}},"description":"handwriting recognition of digits","id":"27","lastUpdateTimeSinceEpoch":"1712076638394","name":"mnist"}

and on the backend:

but likely the current discussed proposal to use "empty valued properties",
eg:

"customProperties":{"my-label1":{"metadataType":"MetadataStringValue","string_value":""},"my-label2":{"metadataType":"MetadataStringValue","string_value":""}}

is proving more simpler to handle

[dhirajsb]

I'd go with the simpler empty string implementation. It's easier to implement, and intuitive. It also works for the scenario you mentioned @tarilabs of properties with optional values. They can semantically (and correctly) be interpreted as labels when a value is not specified.

[rareddy]

can there be a key value pair with empty string in custom properties perhaps?

another suggestion, what if u introduce another discriminator called "label"? will that work?

[tarilabs]

can there be a key value pair with empty string in custom properties perhaps?

another suggestion, what if u introduce another discriminator called "label"? will that work?

the first I believe is exactly what was discussed in the earlier messages, can you confirm?
I didn't understand how to manage the latter one in MLMD, since we can't modify MLMD easily.

fwiw, added demo with kubeflow/model-registry#53

[rareddy]

Thanks @tarilabs sounds good.

[rareddy]

is there any further implementation done for this or can this be closed?

[tarilabs]

is there any further implementation done for this or can this be closed?

we need kubeflow/model-registry#53 merged for this to be closed.

[rareddy]

I am good with that, can I lgtm or do you need a review by anyone else?

[tarilabs]

I am good with that, can I lgtm or do you need a review by anyone else?

as soon as kubeflow/model-registry#53 is merged, I will make sure this one is closed as well for https://issues.redhat.com/browse/RHOAIENG-893

[tarilabs]

done with kubeflow/model-registry#53