compose plugin id #14063

sparkoo · 2019-07-30T06:46:57Z

Signed-off-by: Michal Vala mvala@redhat.com

What does this PR do?

Compose plugin id in ExtendedPluginFQN and ChePlugin when all parts are known. First caused NPE, second that task was not properly configured to one particular component, but was present in all components.

What issues does this PR fix or reference?

#13842

How to test

here's simple devfile

---
apiVersion: 1.0.0
metadata:
  name: task-in-theia-test
components:
  -
    type: dockerimage
    alias: golang
    image: quay.io/eclipse/che-golang-1.10:nightly
    memoryLimit: 64Mi
  - 
    alias: theia-editor
    reference: >-
      https://github.com/raw/eclipse/che-plugin-registry/master/v3/plugins/eclipse/che-theia/7.0.0-next/meta.yaml
    type: cheEditor
commands:
  -
    name: golang task
    actions:
      -
        type: exec
        component: golang
        command: "echo hello from golang component && ls /"
  -
    name: theia task
    actions:
      -
        type: exec
        component: theia-editor
        command: "echo hello from theia component && ls /"

che-server image with PR changes quay.io/mvala/che-server:npefix

musienko-maxim · 2019-07-30T06:46:58Z

Can one of the admins verify this PR?

sparkoo · 2019-07-30T07:38:07Z

ci-build

...-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/ChePlugin.java

sleshchenko

In general, looks good.
Please consider adding java doc to PluginFqn, ExtendedFqn and describe that getId() and other methods may return null and when.

Also, since getId() may return null, maybe it makes sense to add null check in invocation places, like this https://github.com/eclipse/che/blob/f601f1652795048641001387c1ef13192f38a224/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/convert/component/editor/EditorComponentToWorkspaceApplier.java#L102?

sparkoo · 2019-07-30T14:42:30Z

@sleshchenko both PluginFQN and ExtendedPluginFQN are just simple entity classes, without any logic. Values are null when you set them null. I've wrote simple doc to the constructor.
about getId usage nullchecks, I think it would distract the PR and is out of scope. The chance of id being null is lower by this patch. If you think it is still good idea, I will do it as separate PR under same issue. I would probably need help to decide what to do if it is null at some places.

sparkoo · 2019-07-30T14:50:44Z

there is still one remaining issue though. Tasks looks like properly assigned to components, but when I try to run one, task selection pops up:

When I choose one, it properly runs.

sleshchenko · 2019-07-31T07:26:24Z

there is still one remaining issue though. Tasks looks like properly assigned to components, but when I try to run one, task selection pops up:

@sparkoo Could you provide Runtime JSON?

sparkoo · 2019-07-31T07:43:21Z

Runtime JSON

"runtime": {
    "machines": {
      "theia-idecdb": {
        "attributes": {
          "memoryLimitBytes": "512000000",
          "memoryRequestBytes": "512000000",
          "source": "tool",
          "plugin": "eclipse/che-theia/next"
        },
        "servers": {
          "theia-dev": {
            "url": "http://servers0k6t8yp-theia-idecdb-server-3130.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "type": "ide-dev",
              "port": "3130",
              "discoverable": "false"
            },
            "status": "UNKNOWN"
          },
          "theia": {
            "url": "http://servers0k6t8yp-theia-idecdb-server-3100.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "port": "3100",
              "discoverable": "false",
              "cookiesAuthEnabled": "true",
              "type": "ide",
              "secure": "true"
            },
            "status": "RUNNING"
          },
          "theia-redirect-3": {
            "url": "http://servers0k6t8yp-theia-idecdb-server-13133.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "port": "13133",
              "discoverable": "false"
            },
            "status": "UNKNOWN"
          },
          "theia-redirect-2": {
            "url": "http://servers0k6t8yp-theia-idecdb-server-13132.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "port": "13132",
              "discoverable": "false"
            },
            "status": "UNKNOWN"
          },
          "theia-redirect-1": {
            "url": "http://servers0k6t8yp-theia-idecdb-server-13131.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "port": "13131",
              "discoverable": "false"
            },
            "status": "UNKNOWN"
          }
        },
        "status": "RUNNING"
      },
      "vscode-javain7": {
        "attributes": {
          "memoryLimitBytes": "1572864000",
          "memoryRequestBytes": "1572864000",
          "source": "tool",
          "plugin": "redhat/java/latest"
        },
        "status": "RUNNING"
      },
      "che-machine-execxhu": {
        "attributes": {
          "memoryLimitBytes": "134217728",
          "memoryRequestBytes": "134217728",
          "source": "tool",
          "plugin": "eclipse/che-machine-exec-plugin/0.0.1"
        },
        "servers": {
          "che-machine-exec": {
            "url": "ws://serverw6uxhkan-che-machine-execxhu-server-4444.192.168.99.105.nip.io/",
            "attributes": {
              "internal": "false",
              "type": "terminal",
              "port": "4444",
              "discoverable": "false"
            },
            "status": "UNKNOWN"
          }
        },
        "status": "RUNNING"
      },
      "mysql": {
        "attributes": {
          "memoryLimitBytes": "268435456",
          "memoryRequestBytes": "268435456",
          "source": "recipe"
        },
        "status": "RUNNING"
      },
      "tools": {
        "attributes": {
          "memoryLimitBytes": "734003200",
          "memoryRequestBytes": "734003200",
          "source": "recipe"
        },
        "servers": {
          "8080/tcp": {
            "url": "http://serveriaa5xtix-tools-server-8080.192.168.99.105.nip.io/",
            "attributes": {
              "port": "8080"
            },
            "status": "UNKNOWN"
          }
        },
        "status": "RUNNING"
      }
    },
    "activeEnv": "default",
    "commands": [
      {
        "commandLine": "mvn clean install",
        "name": "maven build",
        "attributes": {
          "componentAlias": "tools",
          "machineName": "tools",
          "workingDir": "${CHE_PROJECTS_ROOT}/web-java-spring-petclinic"
        },
        "type": "exec"
      },
      {
        "commandLine": "SPRING_DATASOURCE_URL=jdbc:mysql://db/petclinic SPRING_DATASOURCE_USERNAME=petclinic SPRING_DATASOURCE_PASSWORD=password java -jar -Dspring.profiles.active=mysql target/*.jar\n",
        "name": "run webapp",
        "attributes": {
          "componentAlias": "tools",
          "machineName": "tools",
          "workingDir": "${CHE_PROJECTS_ROOT}/web-java-spring-petclinic"
        },
        "type": "exec"
      },
      {
        "commandLine": "/opt/rh/rh-mysql57/root/usr/bin/mysql -u root < ${CHE_PROJECTS_ROOT}/web-java-spring-petclinic/src/main/resources/db/mysql/schema.sql &&\necho -e \"\\e[32mDone.\\e[0m Database petclinic was configured!\"\n",
        "name": "prepare database",
        "attributes": {
          "componentAlias": "mysql",
          "machineName": "mysql"
        },
        "type": "exec"
      },
      {
        "commandLine": "echo hello",
        "name": "testtest",
        "attributes": {
          "componentAlias": "theia-editor",
          "plugin": "eclipse/che-theia/next",
          "machineName": "theia-idecdb"
        },
        "type": "exec"
      }
    ],
    "machineToken": ""
  },

sparkoo · 2019-07-31T08:17:59Z

ci-build

sparkoo · 2019-07-31T08:18:04Z

ci-test

skabashnyuk · 2019-07-31T08:27:40Z

ci-test

che-bot · 2019-07-31T09:10:31Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14063
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

sparkoo · 2019-07-31T12:44:35Z

ci-test

sparkoo · 2019-07-31T13:18:53Z

I've updated PR to compose plugin id at it's creation time rather than in getter. I'd rather keep entity class clean as possible.

che-bot · 2019-07-31T13:27:58Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14063
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

...ce/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/ExtendedPluginFQN.java

l0rd · 2019-07-31T13:50:10Z

Do not merge to master, the related issue is added to milestone 7.1.0

sparkoo · 2019-07-31T14:52:44Z

Do not merge to master, the related issue is added to milestone 7.1.0

Do we have an estimate when 7.0.0 will happen? It would be unfortunate if we have many PRs in this "do not merge yet" state.

l0rd · 2019-07-31T15:29:51Z

@sparkoo we will do the release as soon as all blockers with milestone 7.0.0 will be fixed. There are only 4 PRs labelled do-not-merge (including this one) so I am not that worried. We can create a 7.1.0 branch and merge those PRs there if the number increase.

But otherwise we should stick to the process. For your info the issue was labelled 7.1.0 a few weeks ago whereas there are still 20+ issues labelled 7.0.0.

che-bot · 2019-08-12T08:32:24Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

build details
"che-server" docker image: maxura/che-server:14063

skabashnyuk · 2019-08-12T08:38:38Z

LGTM. Is there a way to add a unit test?

che-bot · 2019-08-12T12:40:25Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

build details
"che-server" docker image: maxura/che-server:14063
test artifacts
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

Signed-off-by: Michal Vala <mvala@redhat.com>

…e doc Signed-off-by: Michal Vala <mvala@redhat.com>

Signed-off-by: Michal Vala <mvala@redhat.com>

che-bot · 2019-08-15T12:03:53Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

build details
"che-server" docker image: maxura/che-server:14063
test artifacts
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

sleshchenko

LGTM

dmytro-ndp · 2019-08-15T13:48:31Z

@sleshchenko, @sparkoo: FYI:
Happy path tests has been failed because of another PR broke Happy path test execution.

skabashnyuk reviewed Jul 30, 2019

View reviewed changes

...-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/ChePlugin.java Outdated Show resolved Hide resolved

sleshchenko reviewed Jul 30, 2019

View reviewed changes

sparkoo marked this pull request as ready for review July 31, 2019 13:17

sparkoo requested review from amisevsk, l0rd, metlos, mshaposhnik, nickboldt and rhopp as code owners July 31, 2019 13:17

amisevsk reviewed Jul 31, 2019

View reviewed changes

...ce/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/ExtendedPluginFQN.java Outdated Show resolved Hide resolved

che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Jul 31, 2019

l0rd added the do-not-merge/hold label Jul 31, 2019

skabashnyuk added this to the 7.1.0 milestone Jul 31, 2019

skabashnyuk approved these changes Aug 12, 2019

View reviewed changes

sparkoo added 7 commits August 15, 2019 13:52

compose plugin id

ae078d9

Signed-off-by: Michal Vala <mvala@redhat.com>

compose plugin id in ChePlugin when null

03c5f21

Signed-off-by: Michal Vala <mvala@redhat.com>

compose plugin ids in more effective way

a3d8edb

Signed-off-by: Michal Vala <mvala@redhat.com>

compose plugin id javadoc

6274404

Signed-off-by: Michal Vala <mvala@redhat.com>

compose plugin id at creation rather than inside getter

c3febad

Signed-off-by: Michal Vala <mvala@redhat.com>

set PluginFQN id with constructor rather than from child-class. updat…

a73acf0

…e doc Signed-off-by: Michal Vala <mvala@redhat.com>

added simple ExtendedPluginFQN constructor test

b79d6b3

Signed-off-by: Michal Vala <mvala@redhat.com>

sleshchenko approved these changes Aug 15, 2019

View reviewed changes

sleshchenko removed the do-not-merge/hold label Aug 15, 2019

sleshchenko merged commit 45f42e9 into eclipse-che:master Aug 15, 2019

sparkoo deleted the NPE-theia-command branch August 15, 2019 13:48

che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compose plugin id #14063

compose plugin id #14063

sparkoo commented Jul 30, 2019 •

edited

Loading

musienko-maxim commented Jul 30, 2019

sparkoo commented Jul 30, 2019

sleshchenko left a comment

sparkoo commented Jul 30, 2019

sparkoo commented Jul 30, 2019

sleshchenko commented Jul 31, 2019

sparkoo commented Jul 31, 2019 •

edited by sleshchenko

Loading

sparkoo commented Jul 31, 2019

sparkoo commented Jul 31, 2019

skabashnyuk commented Jul 31, 2019

che-bot commented Jul 31, 2019

sparkoo commented Jul 31, 2019

sparkoo commented Jul 31, 2019

che-bot commented Jul 31, 2019

l0rd commented Jul 31, 2019

sparkoo commented Jul 31, 2019

l0rd commented Jul 31, 2019

che-bot commented Aug 12, 2019

skabashnyuk commented Aug 12, 2019

che-bot commented Aug 12, 2019

che-bot commented Aug 15, 2019

sleshchenko left a comment

dmytro-ndp commented Aug 15, 2019 •

edited

Loading

compose plugin id #14063

compose plugin id #14063

Conversation

sparkoo commented Jul 30, 2019 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

How to test

musienko-maxim commented Jul 30, 2019

sparkoo commented Jul 30, 2019

sleshchenko left a comment

Choose a reason for hiding this comment

sparkoo commented Jul 30, 2019

sparkoo commented Jul 30, 2019

sleshchenko commented Jul 31, 2019

sparkoo commented Jul 31, 2019 • edited by sleshchenko Loading

sparkoo commented Jul 31, 2019

sparkoo commented Jul 31, 2019

skabashnyuk commented Jul 31, 2019

che-bot commented Jul 31, 2019

sparkoo commented Jul 31, 2019

sparkoo commented Jul 31, 2019

che-bot commented Jul 31, 2019

l0rd commented Jul 31, 2019

sparkoo commented Jul 31, 2019

l0rd commented Jul 31, 2019

che-bot commented Aug 12, 2019

skabashnyuk commented Aug 12, 2019

che-bot commented Aug 12, 2019

che-bot commented Aug 15, 2019

sleshchenko left a comment

Choose a reason for hiding this comment

dmytro-ndp commented Aug 15, 2019 • edited Loading

sparkoo commented Jul 30, 2019 •

edited

Loading

sparkoo commented Jul 31, 2019 •

edited by sleshchenko

Loading

dmytro-ndp commented Aug 15, 2019 •

edited

Loading