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

Theia should not offer to start apps for debugging #14091

Closed
amisevsk opened this issue Jul 31, 2019 · 18 comments
Closed

Theia should not offer to start apps for debugging #14091

amisevsk opened this issue Jul 31, 2019 · 18 comments
Assignees
Labels
area/editor/theia Issues related to the che-theia IDE of Che kind/enhancement A feature request - must adhere to the feature request template. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.

Comments

@amisevsk
Copy link
Contributor

amisevsk commented Jul 31, 2019

Is your enhancement related to a problem? Please describe.

Many default suggested debug configurations in Theia operate similarly to VSCode: they start the specified app and connect debugging to it.

This does not make sense for Eclipse Che. We often start the Theia sidecar container with a limited (512MiB) memory quota, and often it's impossible to run anything else significant in this container.

Some examples that come to mind are

  1. The netcoredbg plugin for .NET workspaces only seems to work locally, so the application has to start in a sidecar container
  2. The built-in autocompletes for debug configurations include "launch" configs, which call e.g. npm debug in the theia container; these usually get killed before starting up completely

Describe the solution you'd like

At the very least, Theia should not suggest configurations that will not work. Ideally Theia would be able to somehow operate in the main workspace container (which should have enough memory to run the app), but I realize this is likely not possible in a general way.

Describe alternatives you've considered

Caveat emptor, I guess?

Additional context

@amisevsk amisevsk added kind/enhancement A feature request - must adhere to the feature request template. area/editor/theia Issues related to the che-theia IDE of Che status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Jul 31, 2019
@slemeur slemeur added this to the 7.1.0 milestone Jul 31, 2019
@slemeur slemeur removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Jul 31, 2019
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Aug 1, 2019
@benoitf benoitf added status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Aug 1, 2019
@sunix
Copy link
Contributor

sunix commented Aug 6, 2019

We plan to let the user installing a vscode extension in any sidecar container (even without FROM eclipse/che-theia-endpoint-runtime). Would that solve the problem ?

@amisevsk
Copy link
Contributor Author

amisevsk commented Aug 8, 2019

It may, but still depends on setup. I need to rewrite this bug description when I have some time.

The basic issue is that the application being developed should only be run in the main workspace container. The only mode of debugging supported by Che in any sustainable way is remote debugging in the main container. In my eyes, all "launch" configs should be hidden in Theia unless absolutely necessary, since end-users should not have to worry about what's running in sidecars.

@tsmaeder
Copy link
Contributor

We plan to let the user installing a vscode extension in any sidecar container (even without FROM eclipse/che-theia-endpoint-runtime). Would that solve the problem ?

It would not: the extension needs to be bundled with everything it needs to run: that is the responsibility of the plugin developer, not of the person making the devfile.

@tsmaeder
Copy link
Contributor

@amisevsk I appreciate the goal you're stating, but I'm not sure what you're suggesting is possible, technically: would you just hide any "launching"-type debug configs that a third party VS Code extension contributes from the debug UI?

@amisevsk
Copy link
Contributor Author

@tsmaeder I agree that it may not be possible/feasible -- I'm not familiar enough with how extension-contributed launches work in Theia. Would it be possible to e.g. disable configs with "request": "launch",? In general Che only supports "remote" debugging, where remote is "a different container", although this causes issues in various languages currently (e.g. Go requires the plugin to run the code to be able to debug).

@gorkem
Copy link
Contributor

gorkem commented Aug 13, 2019

@amisevsk Do you think process namespace sharing would help here?

@tsmaeder tsmaeder mentioned this issue Aug 14, 2019
24 tasks
@tsmaeder
Copy link
Contributor

We will do an investigation into:

  1. if it's reasonably feasible to hide certain types of launch configs (the launch configs themselves are in a file, the user can do what she wants) in the UI and when suggesting
  2. Find out how various debug adapters launch their debug processes and if redirecting those launches to the dev-machine is feasible.

@amisevsk
Copy link
Contributor Author

@gorkem From a cursory read, it would at least enable more debug configs to function, since currently "attach to process" style debugs do not work. That would go a long way to alleviating a significant potential source of confusion to new users (who would have to know that theia and their project are running in separate containers).

One concern is that systemd-based containers may fail to run: https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/#understanding-process-namespace-sharing

@tsmaeder
Copy link
Contributor

@gorkem since we can't launch anything in the dev container, attaching usually does not even come into play. But yes, it would enable more scenarios in the future.

@tolusha
Copy link
Contributor

tolusha commented Aug 16, 2019

@tsmaeder

it's reasonably feasible to hide certain types of launch configs

Is it ok to remove them from the file once launch.json is saved?

@tsmaeder
Copy link
Contributor

No, I don't think so: tools should never lose data a user may have written by hand.

@tolusha
Copy link
Contributor

tolusha commented Aug 16, 2019

So, what does hide mean then?

@amisevsk
Copy link
Contributor Author

@tolusha I'm generally referring to the autocompletes when you're adding configurations -- either through the "Add configuration" option or by using autocompletion in launch.json.

@tolusha
Copy link
Contributor

tolusha commented Aug 19, 2019

@tsmaeder @amisevsk
I've just checked [1] and it works fine.
It is pretty simple to hide launch configurations from user. At the same time user is able to add them manually.

[1] https://github.com/eclipse/che-theia/blob/ab/filterDebugConf/extensions/eclipse-che-theia-plugin-remote/src/node/debug-configuration-filter.ts

@amisevsk
Copy link
Contributor Author

@tolusha I think it's fine if the user is adding manually, I'm more concerned with che-theia leading users down a path that ultimately will not work for them.

@tsmaeder tsmaeder mentioned this issue Aug 29, 2019
41 tasks
@tsmaeder
Copy link
Contributor

tsmaeder commented Sep 3, 2019

This needs some design work: just filtering out all launch type configurations seems excessive. does it make sense to filter based on some configuration in the plugin metadata?

@tsmaeder
Copy link
Contributor

tsmaeder commented Sep 3, 2019

Idea from @tolusha : pass a list of launch config names through an environment variable to the theia ide container. This would solve the problem from devfiles, but not from plugin metadata. We'd have to have a way to pass environment variables to the theia container.

@tsmaeder tsmaeder modified the milestones: 7.1.0, Backlog - Languages Sep 23, 2019
@tsmaeder tsmaeder mentioned this issue Sep 24, 2019
30 tasks
@tsmaeder tsmaeder mentioned this issue Oct 11, 2019
26 tasks
@che-bot
Copy link
Contributor

che-bot commented Mar 25, 2020

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 25, 2020
@che-bot che-bot closed this as completed Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/editor/theia Issues related to the che-theia IDE of Che kind/enhancement A feature request - must adhere to the feature request template. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.
Projects
None yet
Development

No branches or pull requests

8 participants