devfile / api

Kube-native API for cloud development workspaces specification
Apache License 2.0
237 stars 58 forks source link

Polymorphic component type UX in a devfile #4

Closed l0rd closed 3 years ago

l0rd commented 4 years ago

Polymorphic types makes devWorkspaces objects easier to validate and more close as a Kubernetes custom resources. But are we sacrificing the UX of the devfile? Was the 1.0.0 devfile spec easier to read and modify for humans?

devfile 1.0.0

components:
  - type: container
    image: maven
      ...
  - type: container
    image: nodejs
      ...
  - type: kubernetes
    reference: https://gist.../mongodb.yaml
       ...

current proposal

components:
  - container:
        image: maven
        ...
  - container:
        image: nodejs
        ...
  - kubernetes:
       reference: https://gist.../mongodb.yaml
       ...

alternative proposal

components:
  containers:
    - image: maven
        ...
    - image: nodejs
        ...
  kubernetes:
    - reference: https://gist.../mongodb.yaml
    ...
johnmcollier commented 4 years ago

I prefer the alternative proposal. Having each type (containers, kubernetes, etc) under component be an array of elements (of length 0, 1, or many), makes things easier to read / understand in my opinion

joshuawilson commented 4 years ago

I was thinking of it a little bit different from the alt version.

Current

components:
  - chePlugin:
      registry:
        id: redhat/vscode-yaml/latest
  - chePlugin:
      registry:
        id: ms-vscode/go/latest
  - cheEditor:
      registry:
        id: eclipse/che-theia/latest
        registryUrl: "external-registry-url"
  - container:
      image: some container image with required build tools
      name: "build-tools" 

Alternative 2

components:
  - component:
      type: chePlugin
      registry:
        id: redhat/vscode-yaml/latest
  - component:
      type: chePlugin
      registry:
        id: ms-vscode/go/latest
  - component:
      type: cheEditor
      registry:
        id: eclipse/che-theia/latest
        registryUrl: "external-registry-url"
  - component:
      type: container
      image: some container image with required build tools
      name: "build-tools" 

Alt 2 provides a more extensible model. If we use the current proposed schema, then any change would need to be done in Go code. However, I can see how this may take more work to validate.

I do not think the language should drive the data structure but it can inform how it is made.

l0rd commented 4 years ago

@joshuawilson this looks redundant:

components:
  - component:
      type: ...

I would rather have the component spec directly without prefixing it with component::

components:
  - type: ...

And that corresponds to the devfile 1.0.0 format (and if there is an agreement that this is the best alternative I am perfectly fine with it).

davidfestal commented 4 years ago

@joshuawilson afaict, the following alt2 syntax proposal ...

components:
  - component:
      type: chePlugin
      registry:
        id: redhat/vscode-yaml/latest
  - component:
      type: cheEditor
      registry:
        id: eclipse/che-theia/latest
        registryUrl: "external-registry-url"
  - component:
      type: container
      image: some container image with required build tools
      name: "build-tools" 

... seems to move the problem encountered with the old Devfile 1.0.0 syntax, to a lower level (under the intermediate component level):

You may have distinct sub-schemas (according to the type field) under the same field name (component), which makes declarative validation (validation by the schema itself) harder it seems and, afaik, makes it very difficult to serialize/unserialize in some languages that don't have polymorphic subtyping (like GO).

OTOH, the whole idea of devfile 2.0.0 current proposal is to avoid having different sub-schemas under the same field name, in a way that would be at the same time:

Some pointers about the context that drove this initial design;

davidfestal commented 4 years ago

I prefer the alternative proposal. Having each type (containers, kubernetes, etc) under component be an array of elements (of length 0, 1, or many), makes things easier to read / understand in my opinion

I assume this adds a constraint on the order in which components have to be described, since they would be gathered by type ?

metlos commented 4 years ago

I am not sure if it is a concern of the current effort for Devfile 2.0 but IMHO the "clustering" by component type, reflected in the schema, makes it kinda hard for the devfile spec to be interoperable, e.g. reused by other "processors" than Che/ODO without the need to add explicit support for them in the source code.

E.g. in my dreamt up world, I would like to support multiple IDEs with a single devfile, maybe using something like this:

components:
- type: chePlugin
  id: redhat/java/latest
- type: eclipseDesktopPlugin
  id: org.eclipse.jdt
- type: gitpodFeature
  id: java-support 
...

Is that something we want to address in any way or is it out of scope?

Or, given the majority of examples above, are we maybe considering externalizing the IDE configuration into some other part of the devfile than components - something in support of https://github.com/che-incubator/devworkspace-api/issues/5?

davidfestal commented 4 years ago

@metlos In your example:

components:
- type: chePlugin
  id: redhat/java/latest
- type: eclipseDesktopPlugin
  id: org.eclipse.jdt
- type: gitpodFeature
  id: java-support 

the 3 components are in fact structurally equivalent, have the same sub-schema, and thus to me they seem to be the same type of component.

The fact that they would define different types of plugin would probably fit into the encapsulated plugin definition itself (currently the meta.yaml format in devfile.1.0.0). Sure we initially called this component type chePlugin, but in fact if you look into the meta.yaml format it already covers 3 flavors of plugins: pure Che plugin (cf che-machine-exec, theia editor), Theia plugin or VsCode plugin.

So it seems to me that, while discussing this matter, we should avoid mixing component type, which drives distinct component sub-schemas, and what a plugin type field could be, i.e. a way to provide more insight about how a given plugin should be installed / used.

l0rd commented 4 years ago

@metlos that's an interesting scenario and I think we should take it in consideration. Your sample with the current proposal would look like:

components:
  - chePlugin:
        id: redhat/java/latest
  - eclipseDesktopPlugin:
        id: org.eclipse.jdt
  - gitpodFeature:
       id: java-support 
...

or, if I am interpreting @davidfestal answer correctly:

components:
  - plugin:
        id: redhat/java/latest
  - plugin:
        id: desktopEclipse/org.eclipse.jdt
  - plugin:
       id: gitpodFeature/java-support 
...

What's wrong with those?

metlos commented 4 years ago

What I didn't take into consideration and what we discussed with @davidfestal before he posted his answer here was that the distinction between the different plugin types (e.g. theia/eclipse/gitpod/codewind/...) can happen in meta.yaml of the plugins and that all we then need is to be "forgiving" when applying such plugins to workspace (e.g. a eclipse IDE plugin in the devfile shouldn't make it impossible to use in Che).

l0rd commented 4 years ago

Discussed and agreed to go with "Current proposal"

davidfestal commented 3 years ago

Implemented