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

[debug] Download vscode debug extensions from elsewhere #6120

Merged
merged 1 commit into from Sep 6, 2019

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Sep 5, 2019

This patch is a minimal fix for #6111, presented as a less extreme alternative to PR #6113. Instead of removing the @theia/debug-nodejs and @theia/java-debug theia extensions without advance warning to our adopters that may be using them, it instead fetches the underlying debug adapters from alternate sources.

It downloads the Java debug adapter from the component's GH releases and node-debug / node-debug2 from our fork of these components, since the .vsix files are not available from upstream repos releases.

The longer-term solution is, as was recently briefly discussed, to have our own public registry for extensions.

Note: The alternate sources are 3 GH repo releases. This is not ideal either since it could exacerbate the "GH API rate limit" issues we have had. However the size of all 3 extensions together is a relatively modest ~4MB, so I think that with a GH token set, it should not cause issues in our CI.

Note2: untested at the time of this PR creation, other than looking at the download folders and confirming that the content appears ok at a glance.

cc: @svenefftinge @akosyakov @AlexTugarev @kittaakos

This patch downloads the Java debug adapter from
the component's GH releases and node-debug /
node-debug2 from our fork of these components,
since the .vsix files are not available from
upstream repos releases.

Fixes #6111

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@marcdumais-work
Copy link
Contributor Author

@marechal-p @vince-fugnitto Could you please test that debug still works for Java and Node?

@AlexTugarev
Copy link
Contributor

@marcdumais-work fetching release artifacts from GitHub isn't considered as API call. you're redirected to some cloud storage hosted e.g. S3. also every API call's response will contain a the current rate count in headers.

Copy link
Contributor

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

Node debugger works fine!

@AlexTugarev
Copy link
Contributor

Oh, just noticed that #6113 is competing with this🥇

Could we agree on one way how to proceed?

@vince-fugnitto
Copy link
Member

Could we agree on one way how to proceed?

I believe this would be the better approach since it is less radical, and will not break clients unexpectedly. Removing the extensions entirely would be the last possible solution in my opinion.
This way apps that referenced the extensions will not be broken, and we keep existing functionality intact.

@marcdumais-work marcdumais-work added debug issues that related to debug functionality vscode issues related to VSCode compatibility labels Sep 5, 2019
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

Let’s merge it and discuss the radical approach on dev meeting? Generally we should also push to use VS Code extensions instead of native.

@akosyakov
Copy link
Member

@marcdumais-work We should change download debug adapters scripts that it picks up GITHUB_TOKEN from process.env and pass it as a HEADER if URL points to github.com, like here: https://github.com/microsoft/vscode-ripgrep/blob/a465bbe5eef4fa1781e0091ad906aef7fa8a7a91/lib/download.js#L105

@AlexTugarev
Copy link
Contributor

@akosyakov, but this is only needed for API requests.

@akosyakov
Copy link
Member

@AlexTugarev ok, though that GitHub has rate limits on any request

@marcdumais-work marcdumais-work merged commit e93b1d4 into master Sep 6, 2019
@marcdumais-work marcdumais-work deleted the alternate_vscode_gbg_exts branch September 6, 2019 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants