Skip to content

Improve ports #440

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

Merged
merged 1 commit into from
Sep 28, 2022
Merged

Improve ports #440

merged 1 commit into from
Sep 28, 2022

Conversation

jeanp413
Copy link
Member

@jeanp413 jeanp413 commented Sep 21, 2022

Related issues

Fix gitpod-io/gitpod#8377
Fix gitpod-io/gitpod#11616

How to test

  1. Open this PR in vscode-desktop, compile (run yarn vscode:prepublish in gitpod-remote folder) and launch "Launch gitpod-remote" debug config
  2. Open a workspace with ports configured in gitpod.yml
  3. Check the ports are forwarded when connecting in ports view
  4. Run some command that uses some port
  5. Check the port is forwarded automatically
  6. Check exposed ports webview view is visible and working as expected

@jeanp413 jeanp413 requested a review from akosyakov September 21, 2022 04:14
@jeanp413
Copy link
Member Author

Seems it won't be possible to add the make public/private inline action as there's no context key we can use it to enable/disabled accordingly

@akosyakov
Copy link
Member

Seems it won't be possible to add the make public/private inline action as there's no context key we can use it to enable/disabled accordingly

because it should be part of item?

@jeanp413
Copy link
Member Author

because it should be part of item?

yeah


type PortCommand = typeof PortCommands[number];

export class GitpodPortViewProvider implements vscode.WebviewViewProvider {
Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicate this for now as it's using configcat code on gitpod-web and migration is almost done, so not worth to move more code I think

Copy link
Member

Choose a reason for hiding this comment

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

@mustard-mh Is it done can we just remove old code with this PR?

@akosyakov
Copy link
Member

akosyakov commented Sep 26, 2022

@jeanp413 Is it good? I tried from sources for VS Code Browser and view is empty. It is the same in VS Code Desktop. Let me know when it is good to try.


type PortCommand = typeof PortCommands[number];

export class GitpodPortViewProvider implements vscode.WebviewViewProvider {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why we cannot move everything to shared, and then pass options for customizations?

There seem to be still quite duplication between remote and web?

@jeanp413
Copy link
Member Author

jeanp413 commented Sep 26, 2022

@akosyakov yeah it works but I forgot to mention how to compile 🤦 , right now is kinda tricky run the yarn compile in extensions folder and then yarn build:webview in gitpod-remote folder, I'll improve the scripts.
For gitpod-web is more tricky to test because webviews don't work out of the box, you'll need to change the webview resources url endpoint manually in workbench.ts

@jeanp413
Copy link
Member Author

Added scripts in gitpod-remote folder:

  • to compile both webview and extension run yarn vscode:prepublish
  • to compile only ext run yarn compile
  • to compile webview run build:webview

const req = new PortsStatusRequest();
req.setObserve(true);
const evts = context.supervisor.status.portsStatus(req, context.supervisor.metadata);
stopUpdates = evts.cancel.bind(evts);
Copy link

@mustard-mh mustard-mh Sep 27, 2022

Choose a reason for hiding this comment

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

What we use stopUpdates for? like cancelContext in golang?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copied this xD, but it will be used in the dispose method to cancel the portsStatus request

@mustard-mh
Copy link

I will test it tomorrow

@mustard-mh
Copy link

Tested it follow the how to test section, desktop vscode works well

VSCode Ports Gitpod Ports
image image

Also build a preview env to test browser vscode, code build process not broken and ports view works too.

image

"panel": [
{
"id": "portsView",
"title": "Exposed Ports",
Copy link

@mustard-mh mustard-mh Sep 28, 2022

Choose a reason for hiding this comment

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

non-block: Change it to Gitpod Ports can make it more clear?

Copy link
Member

Choose a reason for hiding this comment

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

No, it does not, ports are not belonging to Gitpod, they are the same ports but in different networks. We should point out to difference in networks. And exposed means exposed on Internet in our docs.

@akosyakov
Copy link
Member

What about the status bar? Should we show ports there as in browser? What view we should open on click? Can we show a menu to select Forwarded ports or Exposed ports? I think it can be another PR though. This one is already big.

@jeanp413
Copy link
Member Author

jeanp413 commented Sep 28, 2022

I'll merge this then and publish a new gitpod-remote version

What about the status bar? Should we show ports there as in browser?

Personally I don't like the ports in status bar as it can take a lot of space and also seems repetitive as we already have two ports view, but we could add it in another PR if needed

@jeanp413 jeanp413 merged commit 5708dcc into gp-code/main Sep 28, 2022
@jeanp413 jeanp413 deleted the jp/improve-ports branch September 28, 2022 17:07
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.

3 participants