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

[POC] tasks and launch configuration as part of Devfile #13057

Closed
skabashnyuk opened this issue Apr 3, 2019 · 21 comments
Closed

[POC] tasks and launch configuration as part of Devfile #13057

skabashnyuk opened this issue Apr 3, 2019 · 21 comments
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system.

Comments

@skabashnyuk
Copy link
Contributor

Description

[POC] tasks and launch configuration as part of devfile

As a part of the solution of these issues

For example like this

tasks:
- label: Client Build
  command: gulp
  args:
  - build
  options:
    cwd: "${workspaceRoot}/client"
- label: Server Build
  command: gulp
  args:
  - build
  options:
    cwd: "${workspaceRoot}/server"
- label: Build
  dependsOn:
  - Client Build
  - Server Build
- label: build2
  command: gcc
  args:
  - "-Wall"
  - helloWorld.c
  - "-o"
  - helloWorld
  problemMatcher:
    owner: cpp
    fileLocation:
    - relative
    - "${workspaceFolder}"
    pattern:
      regexp: "^(.*):(\\d+):(\\d+):\\s+(warning|error):\\s+(.*)$"
      file: 1
      line: 2
      column: 3
      severity: 4
      message: 5
launch-configurations:
- type: node
  request: launch
  name: Server
  program: "${workspaceFolder}/server.js"
  cwd: "${workspaceFolder}"
- type: node
  request: launch
  name: Client
  program: "${workspaceFolder}/client.js"
  cwd: "${workspaceFolder}"

The goal of this task would be a study of:

and propose some variants

@skabashnyuk skabashnyuk added kind/task Internal things, technical debt, and to-do tasks to be performed. team/platform severity/P1 Has a major impact to usage or development of the system. labels Apr 3, 2019
@metlos
Copy link
Contributor

metlos commented Apr 8, 2019

VSCode has these abilities in tasks

  • name of the task
  • cwd of the task
  • command to run
  • add OS specific configuration - not really needed for us
  • execute command in a shell or directly as a process - not sure if it makes a difference in our case
  • execute command in the background or foreground - probably not important
  • args of the command - not sure if that is really needed given how we execute the command
  • override env vars for the command
  • specify the shell in which to execute the command - probably not critical to have but can be useful
  • control the presentation of the command output
    • Whether to show the output
    • Whether the output takes focus
    • How to open the panel with the output
  • problem matcher - a way of finding problems in the task output, can be “inherited” and specialized. Has ways to extract the location of the identified problem in the source code (by matching regexp on the output and extracting info from it using match groups) - while this is very nice, I don’t think it is essential to have
  • tasks can be run either explicitly or on opening a folder containing the tasks.json file - not relevant to devfile IMHO
  • a task can either be in a “build” group or “test” group and can be declared the default task for the given groups

VSCode options for launch.json

Different types of run/debug servers support different sets of options
Every launch config supports:

  • name - the human readable name of the configuration
  • type - type of the debugger/launcher to use
  • request - either “launch” or “attach”
  • preLaunchTask - task from tasks.json to run before launch
  • postDebugTask
  • serverReadyAction - a pattern to match on the stdout of the program and an action to execute when the pattern is matched - open a web browser with an URI constructed from the matched pattern, if chrome debugger extension is installed, the action can also be “debugWithChrome”.

Commonly supported options are:

  • program - the executable or file to run
  • args - the arguments to pass to the program
  • env
  • cwd
  • port
  • stopOnEntry

@metlos
Copy link
Contributor

metlos commented Apr 9, 2019

Given the summary above, as a first draft, I propose the following changes.

  1. I think we should drop the notion of multiple actions per command. VS Code doesn't support that as far as I can tell and so we'd have trouble integrating such a thing into Theia, too. If needed in the future, we could introduce new types of command, like sequence or parallel.
  2. In the light of devfile as CRD we should drop the type of the command and instead opt for explicit child objects.
commands:
-   name: build
    component: component
    build:
        cwd: sadf
        command: asdf
        args: [ ... ]
        env:
            key: value
        shell: asdf
        default: bool
    test:
        ... the same props as build ...
    launch:
        type: debugger to use
        attributes:
            key: value
            key:
               complex: value
            ... this is a free-form map-of-any to support the per-debugger type attributes.   
    attach:
        ... same props as launch ...

Of course, the above just lists all the possible command types. A concrete command definition would look more like:

commands:
- name: che-theia ... build
  build:
    cwd: ${workspaceDir}/che-theia
    command: yarn
    default: true

Compared to VS code the above DOESN'T support:

  • control of presentation of output - we just unconditionally open a terminal window
  • problem matcher - this is a good candidate for later improvements but IMHO is not essential ATM
  • pre-launch on post-debug tasks - again, a good candidate for later improvements
  • serverReadyAction - another improvements candidate

@skabashnyuk
Copy link
Contributor Author

skabashnyuk commented Apr 12, 2019

@l0rd @slemeur We have two conceptual issues in the horizont.
Unstructual data like vscode task.json or lounch.json make dificulties in
http -> DTO -> REST -> DB and DB->REST->DTO-> HTTP line aswell it makes troubles
if we want to make Devfile K8s-compliant #13061

Basically, we can try to put such structures as task.json or laounch.json in yaml as a string. Something like

commands:
  - name: vscode
    actions:
      - type: vscode-launch.json 
        component: jdt.ls
        configurations: |
          - type: node
            request: launch
            name: Gulp Build
            program: "${workspaceFolder}/node_modules/gulp/bin/gulp.js"
            stopOnEntry: true
            args:
            - hygiene
          - type: node
            request: attach
            name: Attach to Extension Host
            port: 5870
            restart: true
            outFiles:
            - "${workspaceFolder}/out/**/*.js"
          - type: node
            request: launch
            name: Launch Smoke Test
            program: "${workspaceFolder}/test/smoke/out/main.js"
            cwd: "${workspaceFolder}/test/smoke"
            timeout: 240000
            port: 9999
            args:
            - "-l"
            - "${workspaceFolder}/.build/electron/Code - OSS.app/Contents/MacOS/Electron"
            outFiles:
            - "${cwd}/out/**/*.js"
            env:
              NODE_ENV: development
              VSCODE_DEV: '1'
              VSCODE_CLI: '1'
      - type: vscode-task.json 
        component: theia
        tasks: |
          - type: npm
            script: watch
            label: Build VS Code
            group: build
            isBackground: true
            presentation:
              reveal: never
            problemMatcher:
              owner: typescript
              applyTo: closedDocuments
              fileLocation:
              - absolute
              pattern:
                regexp: 'Error: ([^(]+)\((\d+|\d+,\d+|\d+,\d+,\d+,\d+)\): (.*)$'
                file: 1
                location: 2
                message: 3
              background:
                beginsPattern: Starting compilation
                endsPattern: Finished compilation
          - type: npm
            script: strict-initialization-watch
            label: TS - Strict Initialization
            isBackground: true
            presentation:
              reveal: never
            problemMatcher:
              base: "$tsc-watch"
              owner: typescript-strict-initialization
              applyTo: allDocuments

or something like @metlos proposed #13057 (comment).
the downside of this approach is that field become hard/close to impossible to validate because it becomes a simple string.
the pros are that we can put anything to it and we don't need to change anything to support any type of vscode plugins configurations.

Another approach is to get a subset of task.json or laounch.json and try to make them somehow K8s-compliant, as a base we can take
https://github.com/Microsoft/vscode/blob/master/.vscode/launch.json
https://github.com/Microsoft/vscode/blob/master/.vscode/tasks.json
the downside of this is that we can't support all possible option of any VSCode plugin and if we need new field we need to change
model.

@l0rd
Copy link
Contributor

l0rd commented Apr 16, 2019

@skabashnyuk @metlos as discussed during today call I would like to avoid to embed the task/launch.json as a huge string. And I would like to avoid the removal of the notion of actions as well (but on that I am more flexible :-)).

One proposal is to define a vs-code-launch-json action as a generic yaml object that can have any field but must be a valid yaml. But I am not sure if JSON schema allows it and its' compatible with a Kubernetes CRD.

Another alternative would be to support a limited task/launch.json through our current model and support it fully through a reference attribute as we do for k8s components.

@metlos
Copy link
Contributor

metlos commented Apr 24, 2019

I agree with @l0rd that embedding the JSON in the devfile YAML is ugly and we should avoid it.

For the problem of actions - I am not sure VS code/Theia natively support tasks consisting of several steps, so this would have to be handled at the task-plugin level which would have to coordinate the execution of the tasks across the components. Not impossible, but also something special to Che, so users are IMHO going to have trouble understanding the interplay between the different components (theia, tasks-plugin, che plugins, che devfile components) involved in coordinating all this. But it is certainly doable IMHO.

I like the idea of reference to point to the editor-specific configuration in editor-specific format. Arguably, that will allow people to reuse (parts of) their existing IDE configuration and use it with Che without needing to learn another configuration format and its limitations.

Therefore I propose this:

# leave commands mostly as is
commands:
- name: test and manual test
  attributes:
    group: test   
  actions:
  - type: exec # I frankly still don't understand what we need this for
    component: mvn-stack
    command: mvn clean package
  - component: runtime
    command: java -jar service.jar
    workdir: $CHE_PROJECTS_ROOT/service/target
# This is new stuff to support editor-specific configuration
editorConfiguration:
- editor: org.eclipse.che.editor.theia:1.0.0
  - type: tasks
    reference: ../ide-config/vs-code/tasks.json
  - type: launch
    reference: ../ide-config/vs-code/launch.json
  - type: formatting
    reference: https://acme.com/developer/vs-code-java-formatting.xml

In the above, the only thing that the devfile author needs to understand is that the commands end up being tasks in Theia. This behavior is actually editor-specific (because I assume each editor is going to "materialize" the commands differently) and so this information can only be documented, not codified in the format IMHO.

I'm not happy with the editorConfiguration needing to specify the editor for which it applies, but this is because we don't require the explicit editor definition in the devfile anymore. If the default editor is different to what the devfile author assumed and we had no way of telling what editor is expected to receive the configuration, we'd end up in trouble.

The editorConfiguration would be handled by the che-specific plugin in the editor (which we need in any case if we want to support commands). The types of the references in the editor configuration would be again editor specific and would instruct the che-specific plugin in the editor how to apply the referenced files.

@l0rd
Copy link
Contributor

l0rd commented Apr 24, 2019

@metlos a few comments

type: exec # I frankly still don't understand what we need this for

This means that we are going to execute the command in a container (i.e. docker exec does). Initially we thought that other types would be start, stop, restart of a workspace component but we never implemented that although I still consider them valuable.

I'm not happy with the editorConfiguration needing to specify the editor for which it applies, but this is because we don't require the explicit editor definition in the devfile anymore. If the default editor is different to what the devfile author assumed and we had no way of telling what editor is expected to receive the configuration, we'd end up in trouble.

I don't understand your concern and the need for editorConfiguration. Let's keep it simple: if the editor is not explicitly defined che-theia will be used. A devfile author should always expect that the default editor is che-theia, no matter on what Che instance his devfile will be run.

The types of the references in the editor configuration would be again editor specific and would instruct the che-specific plugin in the editor how to apply the referenced files.

Yes that's why I would rather use types vscode-task and vscode-launch. And those types should be at the same level of the exec. In the components we do something similar and use che-plugin and che-editor for che specific components (and we want the devfile to be used by development tools other than Che).

@metlos
Copy link
Contributor

metlos commented Apr 25, 2019

type: exec # I frankly still don't understand what we need this for

This means that we are going to execute the command in a container (i.e. docker exec does). Initially we thought that other types would be start, stop, restart of a workspace component but we never implemented that although I still consider them valuable.

👍 thanks for the explanation

I don't understand your concern and the need for editorConfiguration.

Where would you place the references to launch/tasks?

Let's keep it simple: if the editor is not explicitly defined che-theia will be used. A devfile author should always expect that the default editor is che-theia, no matter on what Che instance his devfile will be run.

Well, the default editor and plugins are configurable, so the admin can actually change that to something else than Theia.

The types of the references in the editor configuration would be again editor specific and would instruct the che-specific plugin in the editor how to apply the referenced files.

Yes that's why I would rather use types vscode-task and vscode-launch.

👍 for prefixing the type names.

And those types should be at the same level of the exec.

How do you mean that?

In the components we do something similar and use che-plugin and che-editor for che specific components (and we want the devfile to be used by development tools other than Che).

If we assume that the devfile is going to be used by other development tools besides Che, can we still assume that the default editor is always going to be Theia?

@l0rd
Copy link
Contributor

l0rd commented Apr 25, 2019

Well, the default editor and plugins are configurable, so the admin can actually change that to something else than Theia.

I think that has been an error. I mean we have made our life more difficult for something that doesn't come from a real user feedback and that doesn't really make sense currently (che-theia is the only real editor atm). That's what I call goldplating and I am probably the one to blame for that. To solve that let's assume that the default editor is always che-theia. The day there will be a real alternative we will discuss this but for now let's forget about it.

How do you mean that?

Something like

# leave commands mostly as is
commands:
- name: test and manual test
  attributes:
    group: test   
  actions:
  - type: exec # I frankly still don't understand what we need this for
    component: mvn-stack
    command: mvn clean package
  - component: runtime
    command: java -jar service.jar
    workdir: $CHE_PROJECTS_ROOT/service/target
- name: run
  actions:
  - type: vscode-task
    component: mvn-stack
    reference: ../ide-config/vs-code/tasks.json
- name: debug
  actions:
  - type: vscode-launch
    component: mvn-stack
    reference: ../ide-config/vs-code/launch.json
- name: format
  actions:
  - type: vscode-formatting
    reference: https://acme.com/developer/vs-code-java-formatting.xml

If we assume that the devfile is going to be used by other development tools besides Che, can we still assume that the default editor is always going to be Theia?

This is a cheEditor so yes it makes sense to assume that default cheEditor is che-theia.

@skabashnyuk
Copy link
Contributor Author

Today after the discussion with @vparfonov @l0rd @metlos @RomanNikitenko @tolusha we had such variants:

Command oriented

commands:
- name: test and manual test
  attributes:
    group: test   
  actions:
  - type: exec # I frankly still don't understand what we need this for
    component: mvn-stack
    command: mvn clean package
  - component: runtime
    command: java -jar service.jar
    workdir: $CHE_PROJECTS_ROOT/service/target
- name: run
  actions:
  - type: vscode-task
    reference: ../ide-config/vs-code/tasks.json
- name: debug
  actions:
  - type: vscode-launch
    reference: ../ide-config/vs-code/launch.json

File oriented

components:
  - alias: mvn-stack
    type: chePlugin
    id: org.eclipse.chemaven-jdk8:1.0.0
  - type: cheEditor
    id: org.eclipse.chetheia:0.0.3
    files:
      - name: ${PROJECT_ROOT}/task.json
        reference: ../ide-config/vs-code/tasks.json
      - name: launch.json
        reference: ../ide-config/vs-code/launch.json

@metlos
Copy link
Contributor

metlos commented Apr 26, 2019

Since I wasn't able to stay until the end of the meeting, here are my 2c on the new proposals:

commands:
- name: test and manual test
  attributes:
    group: test   
  actions:
  - type: exec
    component: mvn-stack
    command: mvn clean package
  - component: runtime
    command: java -jar service.jar
    workdir: $CHE_PROJECTS_ROOT/service/target
- name: run
  actions:
  - type: vscode-task
    reference: ../ide-config/vs-code/tasks.json

I understand the lack of the component here as a suggestion that we will be able to deduce the component to run a task from the information in tasks.json. How are we going to do that? Can we use the task type and specify a different value than process or shell prescribed by the tasks schema (https://code.visualstudio.com/docs/editor/tasks-appendix)?

- name: debug
  actions:
  - type: vscode-launch
    reference: ../ide-config/vs-code/launch.json

as a general remark, I don't particularly like the fact that the reference to a "command file" is embedded in actions. It seems to suggest that there can be more such actions for a single command but I don't think that makes sense. IMHO, the less "overloading" in the schema, the better.

what about:

commands:
  list:
  - name: test and manual test
    attributes:
      group: test   
    actions:
    - type: exec # I frankly still don't understand what we need this for
      component: mvn-stack
      command: mvn clean package
    - component: runtime
      command: java -jar service.jar
      workdir: $CHE_PROJECTS_ROOT/service/target  
  external:
  - name: run
    type: vscode-task
    reference: ../ide-config/vs-code/tasks.json

I'm not sure the file-oriented approach would not limit us in the future because it makes it hard to define tasks that would span several components if I understand it correctly.

@RomanNikitenko
Copy link
Member

I understand the lack of the component here as a suggestion that we will be able to deduce the component to run a task from the information in tasks.json. How are we going to do that? Can we use the task type and specify a different value than process or shell prescribed by the tasks schema

At the moment for Che tasks we register own runner and specify 'che' type for our tasks. So when theia tries to run Che task it finds runner by type and redirects running of a task to our runner.
Che task provider specifies machineName - so we give it with command line to machine exec. We ask user to select container when 'machineName' is not specified.
All described things are related only for Che tasks and our runner.
I need to investigate how we can specify/select container when we run not Che task

@metlos
Copy link
Contributor

metlos commented May 6, 2019

I decided not to pursue the file-based approach, i.e. to reference the files directly with the components.
I see 2 problems with this approach:

  • we would be defining actions and commands on 2 distinct places in the devfile
  • it is basically a general purpose way of submitting generic files to component containers. This opens up the door to basically modify the image of the component container in an arbitrary way.

For the above reasons I think the file-based approach is too broad for the feature that we're trying to implement here. We might find it useful in the future but it should be specifically for the purpose of the runtime image modification - and that will require more thought about what should and should not be allowed.

@RomanNikitenko
Copy link
Member

So, commands section will look like:

commands:
- name: test
  actions:
  - type: exec
    component: golang
    command: "cd $GOPATH/src/github.com/eclipse/che-plugin-broker; go test -v -race ./..."    
- name: run
  actions:
  - type: vscode-task
    referenceContent: |
            {
                    "version": "2.0.0",
                    "tasks": 
                    [
                     {
                      "label": "build",
                       "source": "che",
                       "type": "shell",
                       "cwd": "/home/rnikitenko/projects/task-provider-plugin",
                       "options": {},
                       "command": "yarn",
                       "args": []
                     }
                    ]
            }
- name: debug
  actions:
  - type: vscode-launch
    referenceContent: |
            {
             "version": "0.2.0",
             "configurations": [
              {
               "type": "node",
               "request": "attach",
               "name": "Attach by Process ID",
               "processId": "${command:PickProcess}"
              },
              {
               "type": "node",
               "request": "launch",
               "name": "Launch with Node.js",
               "program": "${file}"
              }
                     ]
            }

We will use type: vscode-task to mark command with content for tasks.json file and type: vscode-launch for launch.json file.
Right?

@RomanNikitenko
Copy link
Member

RomanNikitenko commented May 7, 2019

About running tasks in a specific container.
We have remotes plugins. I turned on the typescript plugin for my workspace and tried to run 'build' and 'watch' tasks for che-theia project, but they failed.
ProcessTaskRunner is used for running typescript task configurations, so tasks for remote plugins are not redirected to container of the plugin at the moment.

update:
Created issue for it https://github.com/eclipse/che-theia/issues/217
It's not related to the way how we provide tasks - I tested it without devfile - just created workspace from Che 7 Theia dev stack and turned on remote typescript plugin.

@metlos
Copy link
Contributor

metlos commented May 7, 2019

I've added a couple of tests and made the PR #13273 ready for review.

@metlos
Copy link
Contributor

metlos commented May 15, 2019

The server-side status is: #13273 is ready, but we need approval on the final format of the devfile before proceeding.

@metlos
Copy link
Contributor

metlos commented May 15, 2019

About running tasks in a specific container.
We have remotes plugins. I turned on the typescript plugin for my workspace and tried to run 'build' and 'watch' tasks for che-theia project, but they failed.
ProcessTaskRunner is used for running typescript task configurations, so tasks for remote plugins are not redirected to container of the plugin at the moment.

The tasks have a type that governs how they are run. I guess it is reasonable to state that the process or shell tasks will run in the theia container. What happens with other task types like "npm"? Is the task still run in the theia container or is actually handed over to the appropriate plugin (which should then run correctly in its own container)?

@RomanNikitenko
Copy link
Member

I created draft PR for #12711. The PR provides ability to export launch configurations in a launch.json file.
Working on ability to export tasks configurations in tasks.json file.

@RomanNikitenko
Copy link
Member

The tasks have a type that governs how they are run. I guess it is reasonable to state that the process or shell tasks will run in the theia container. What happens with other task types like "npm"? Is the task still run in the theia container or is actually handed over to the appropriate plugin (which should then run correctly in its own container)?

I tested only tasks for typescript remote plugin, but I guess the behavior is the same for all remote plugins. Maybe somebody who worked on remote plugins can provide more info about this case

@l0rd
Copy link
Contributor

l0rd commented May 26, 2019

@metlos @skabashnyuk can we close this issue? Have we added some examples of task and launch references somewhere?

@metlos
Copy link
Contributor

metlos commented May 27, 2019

There is still https://github.com/eclipse/che-theia/issues/217 but that needs to be figured out on the UI side first. We may need to revisit the approach implemented here but that can be done in a different issue. Closing this since the functionality is in che master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

4 participants