InseeFrLab / onyxia

🔬 Data science environment for k8s
https://onyxia.sh
MIT License
458 stars 80 forks source link

Service launcher configuration tabs are ordered depending on the values.schema.json #786

Open Eldrile opened 6 months ago

Eldrile commented 6 months ago

Hi, At the time I can't understand the order of the configuration tabs present in a service. It would be great if these tabs could be ordered following the order present in the values.schema.json.

I tried looking into it from the front-end side but while I did find the function getting these values they are already in a different order, any ideas? Thanks !

olevitt commented 6 months ago

Hi !

The spec for values.schema.json defines the "tabs" as JSON objects and not JSON array (see https://github.com/InseeFrLab/helm-charts-interactive-services/blob/main/charts/jupyter-pytorch/values.schema.json for example) so there is no ordering built-in the spec (json objects are key-indexed, not position-indexed). Additionally, in the case of Onyxia, the values.schema.json is read and parsed server-side (onyxia-api) before being sent to the UI (see catalogs endpoints) so if we wanted to preserve the order (which I don't think we should as it's contrary to JSON spec and ideology) we would have to ensure it's done multiple times : at API unmarshalling, at API endpoint marshalling in addition to UI side.
I think if we wanted to implement any sort of fine control over the ordering then we would have to add an additional attribute to the values.schema.json structure. Not sure if that's worth it but it may be up for debate

garronej commented 6 months ago

@olevitt You are correct in principle, but I think it's important to give the author of the charts the ability to enforce an order in the tabs. We could, of course, add a new x-onyxia parameter, but in my opinion, it would be worth it to see why the tabs are getting shuffled on the Java side. Because even if it's not in the spec, every implementation of JSON and JavaScript runtime respects the ordering of keys in object declarations.

Object.keys({ a: "foo", b: "bar" }) // Will always return [ "a", "b" ], never [ "b", "a" ]

Also:

const obj = { a: "foo", b: "bar" };
// and
const obj2 = JSON.parse(`{ "a": "foo", "b": "bar" }`)

are equivalent.

fcomte commented 4 months ago

I guess we should try to respect the order in the schema. Onyxia does not need to parse it. In my mind , in the near futur we will have a specific endpoint to request many json schema.. The one from the catalog could be override by a region. https://github.com/InseeFrLab/onyxia/discussions/798

So Onyxia-web should try to respect the order and Onyxia-api also. For Onyxia-api we should create specific endpoint to provide json schemas.