InseeFrLab / onyxia

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

Support overriding more fields with values from X-Onyxia #819

Closed nicolst closed 3 months ago

nicolst commented 4 months ago

Background

Currently we have the option to override a field's default value using .x-onyxia.overwriteDefaultWith:

"myField": {
  "type": "string",
  "default": "",
  "x-onyxia": {
    "overwriteDefaultWith": "some.attribute"
  }
}

As well as an option to specify which region's config to use for slider values. This is very useful, but in some cases a bit lacking.

Proposal

In some cases it could be useful to override/populate other properties of a field as well. For example, we would like to have a drop-down choice which is populated with a user's groups, which is included as a claim in their ID token:

"group": {
  "type": "string",
  "default": "",
  "render": "list",
  "listEnum": []
}

In this case, we want to override both the default value (to say, the first group in the user's list of groups), and the listEnum property. Two examples of how this could look:

Following the current naming scheme

"group": {
  "type": "string",
  "default": "",
  "render": "list",
  "listEnum": [],
  "x-onyxia": {
    "overwriteDefaultWith": "user.decodedIdToken.groups[0]",
    "overwriteListEnumWith": "user.decodedIdToken.groups"
  }
}

More general and extendable structure?

"group": {
  "type": "string",
  "default": "",
  "render": "list",
  "listEnum": [],
  "x-onyxia": {
    "overrides": {
      "default": "user.decodedIdToken.groups[0]",
      "listEnum": "user.decodedIdToken.groups"
    }
  }
}
fcomte commented 4 months ago

I prefer to keep the same structure to avoid migration of all charts.

garronej commented 4 months ago

I like this proposal. It's simple enough it adresses a valid usecase and the first option is retrocompatible

trygu commented 3 months ago

I like this proposal. It's simple enough it adresses a valid usecase and the first option is retrocompatible

Very nice, @garronej!

Any idea when we could get this in Onyxia? It's a bit of a blocker for us for our grand plan on going into production this summer. <3

garronej commented 3 months ago

Hey @trygu,
I'm shipping this today

trygu commented 3 months ago

Hey @trygu, I'm shipping this today

Did I mention that we love you? ❤️

garronej commented 3 months ago

Sorry I wasn't able to do it today but tomorrow for sure!

garronej commented 3 months ago

It's done. Allow me a couple of hours to test hedge cases and document the feature and I'll ship

garronej commented 3 months ago

@nicolst @trygu It's released! :)

https://docs.onyxia.sh/admin-doc/catalog-of-services#x-onyxia-overwritelistenumwith

fcomte commented 3 months ago

i reopen it as onyxia-api does not handle the new field

garronej commented 3 months ago

Oh gosh I forgot that the values are not forwareded as it by the API. Good catch!

garronej commented 3 months ago

This is released