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

Node debug #666

Merged
merged 4 commits into from
Nov 11, 2020
Merged

Node debug #666

merged 4 commits into from
Nov 11, 2020

Conversation

vitaliy-guliy
Copy link
Contributor

@vitaliy-guliy vitaliy-guliy commented Nov 5, 2020

What does this PR do?

Add

  • ms-vscode/node-debug2 v1.42.5
  • ms-vscode/node-debug v1.44.8

How to test this PR?

Use following devfile to test this PR

---
apiVersion: 1.0.0
metadata:
  generateName: nodejs-
projects:
  -
    name: nodejs-web-app
    source:
      type: git
      location: "https://github.com/che-samples/web-nodejs-sample.git"
components:
  -
    type: chePlugin
    id: che-incubator/typescript/latest
    memoryLimit: 512Mi
  - 
    type: chePlugin
    # id: ms-vscode/node-debug2/latest
    reference: >-
      https://raw.githubusercontent.com/eclipse/che-plugin-registry/node-debug/v3/plugins/ms-vscode/node-debug2/1.42.5/meta.yaml
    preferences:
      debug.node.useV3: false
  -
    type: dockerimage
    alias: nodejs
    image: quay.io/eclipse/che-nodejs10-ubi:nightly
    memoryLimit: 512Mi
    endpoints:
      - name: 'nodejs'
        port: 3000
    mountSources: true
commands:
  -
    name: download dependencies
    actions:
      - type: exec
        component: nodejs
        command: npm install
        workdir: ${CHE_PROJECTS_ROOT}/nodejs-web-app/app
  -
    name: run the web app
    actions:
      - type: exec
        component: nodejs
        command: nodemon app.js
        workdir: ${CHE_PROJECTS_ROOT}/nodejs-web-app/app
  -
    name: run the web app (debugging enabled)
    actions:
      - type: exec
        component: nodejs
        command: nodemon --inspect app.js
        workdir: ${CHE_PROJECTS_ROOT}/nodejs-web-app/app
  -
    name: stop the web app
    actions:
      - type: exec
        component: nodejs
        command: >-
          node_server_pids=$(pgrep -fx '.*nodemon (--inspect )?app.js' | tr "\\n" " ") &&
          echo "Stopping node server with PIDs: ${node_server_pids}" && 
          kill -15 ${node_server_pids} &>/dev/null && echo 'Done.'
  -
    name: Attach remote debugger
    actions:
    - type: vscode-launch
      referenceContent: |
        {
          "version": "0.2.0",
          "configurations": [
            {
              "type": "node",
              "request": "attach",
              "name": "Attach to Remote",
              "address": "localhost",
              "port": 9229,
              "localRoot": "${workspaceFolder}",
              "remoteRoot": "${workspaceFolder}"
            }
          ]
        }

I want to pay attention, that to make node-debug plugin working properly, the workspace must have preference property debug.node.useV3 set to false. For that we should update all the devfiles, that use node-debug plugin.

debug.node.useV3: false

Screenshot/screencast of this PR

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 5, 2020

@vitaliy-guliy Happy Path PR check [build 387] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/387/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/387/

Depending on failure reason, the artifacts or deployment may not be present.

Copy link
Contributor

@ericwill ericwill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add a new meta.yaml, we are only keeping one version of each plugin in the registry now. You can just update the existing meta.yaml with the new version.

@vitaliy-guliy
Copy link
Contributor Author

No need to add a new meta.yaml, we are only keeping one version of each plugin in the registry now. You can just update the existing meta.yaml with the new version.

We have to add it, it's because this is a new 1.42.5 version.

And also I would like to add new meta.yaml for new version, otherwise versioning does not make any sence.
We can always cleanup old versions and just leave few latest.

@ericwill
Copy link
Contributor

No need to add a new meta.yaml, we are only keeping one version of each plugin in the registry now. You can just update the existing meta.yaml with the new version.

We have to add it, it's because this is a new 1.42.5 version.

And also I would like to add new meta.yaml for new version, otherwise versioning does not make any sence.
We can always cleanup old versions and just leave few latest.

We are only keeping the latest version of every plugin now, see eclipse-che/che#18183. So it's not necessary to add new meta.yaml files any longer, we only need to update the single meta.yaml file for each plugin.

@vitaliy-guliy
Copy link
Contributor Author

We are only keeping the latest version of every plugin now, see eclipse/che#18183. So it's not necessary to add new meta.yaml files any longer, we only need to update the single meta.yaml file for each plugin.

Ok, but we still need should rename a directory which indicates the plugin version.

And then if we don't have versions (which is strange, but OK), we don't need to have that directory.
So, the plugin ID become

ms-vscode/node-debug2

instead

ms-vscode/node-debug2/latest

I think it's for feature.

@ericwill
Copy link
Contributor

We are only keeping the latest version of every plugin now, see eclipse/che#18183. So it's not necessary to add new meta.yaml files any longer, we only need to update the single meta.yaml file for each plugin.

Ok, but we still need should rename a directory which indicates the plugin version.

And then if we don't have versions (which is strange, but OK), we don't need to have that directory.
So, the plugin ID become

ms-vscode/node-debug2

instead

ms-vscode/node-debug2/latest

I think it's for feature.

Yes for now just keep the version in the folder name, we can play around with folder names in future PRs.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 10, 2020

@vitaliy-guliy Happy Path PR check [build 399] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/399/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/399/

Depending on failure reason, the artifacts or deployment may not be present.

@che-bot
Copy link
Contributor

che-bot commented Nov 10, 2020

@vitaliy-guliy Happy Path PR check [build 398] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/398/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/398/

Depending on failure reason, the artifacts or deployment may not be present.

@vitaliy-guliy
Copy link
Contributor Author

Now it looks like just updating of existent meta.yaml

Copy link
Contributor

@ericwill ericwill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that we'll need to build the extensions and host them on download.jboss.org for the CRW version of this PR.

@vitaliy-guliy
Copy link
Contributor Author

Please note that we'll need to build the extensions and host them on download.jboss.org for the CRW version of this PR.

What about download them from Open VS Registry?

@ericwill
Copy link
Contributor

Please note that we'll need to build the extensions and host them on download.jboss.org for the CRW version of this PR.

What about download them from Open VS Registry?

CRW product requirements mean we have to build the extension ourselves, and use that.

@ericwill ericwill merged commit 58cba2e into master Nov 11, 2020
@ericwill ericwill deleted the node-debug branch November 11, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants