Skip to content

[JetBrains EAP] Port Forwarding improvements #14408

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
Nov 11, 2022

Conversation

felladrin
Copy link
Contributor

@felladrin felladrin commented Nov 3, 2022

Description

This PR brings the following changes:

  • We now rely on PerClientPortForwardingService for controlling all Ports (Forwarded + Exposed) in JetBrains Client. For this reason, GitpodPortsService was removed.
  • GitpodCLIService.kt and GitpodClientProjectSessionTracker.kt were updated because they relied on GitpodPortsService.
  • Whenever Supervisor informs the client there was a change in the Name, Description, Visibility, Served or Exposed status of a port, Ports View is updated accordingly.
  • GitpodPortForwardingService became an interface, which is implemented only by the Latest package. It's not available for Stable.
  • extractLocalHostUriMetaDataForPortMapping moved to an object (LocalHostUri) as it's a pure function with its test suit. Leaving it inside a Service would make it hard to instantiate and test.
  • GitpodGlobalPortForwardingService was removed as we shouldn't use JetBrains' GlobalPortForwardingService. This is intended for plugins running in local mode instead of remote.

This is how the Ports View currently looks like:
image

Notes:

  • By default, when trying to forward a port which is already in use on Client, a random port will be forwarded (and we take it into account when fetching the forwarded port on client).
    image
    image
  • GitpodPortForwardingService is only available when using Latest (EAP) IDEs, that's why we use serviceOrNull<GitpodPortForwardingService>() when fetching that service.
  • Currently, there's no way to sort the ports in Ports View. They appear in the same order they're registered.

Related Issue(s)

How to test

  1. Open the Preview Environment of this PR and start any workspace using EAP IntelliJ IDEA.
  • Run python -m http.server 3000 and confirm that:
    • a notification is displayed informing that the port is available. Then click "Open browser" and confirm it opens http://127.0.0.1:3000.
    • port 3000 is displayed on the Ports View twice: one with the forwarded address and another with the exposed URL.
    • Copy Web URL and Copy URL dropdown options are working:
      • image
      • image
  • In another terminal run gp ports visibility 3000:public and confirm if in Ports View the Exposed Port doesn't have the lock icon anymore.
  1. Open the Preview Environment of this PR and start any workspace using Stable IntelliJ IDEA.
  • Run python -m http.server 3000 and confirm that a notification appears with a link to "Open browser". Click the link and check if it opens a Gitpod URL, as there's no port forwarding in the Stable version.

Release Notes

NONE

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@felladrin felladrin self-assigned this Nov 3, 2022
@felladrin felladrin changed the title [JetBrains] Improve port forwarding in EAP IDEs [JetBrains EAP] Improve port forwarding Nov 3, 2022
@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch 8 times, most recently from e4e753a to 0aa4e3a Compare November 7, 2022 12:07
@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch 4 times, most recently from 0ffbd2d to 49dcbb6 Compare November 8, 2022 10:27
@werft-gitpod-dev-com

This comment was marked as off-topic.

@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch from 49dcbb6 to d3c8a2d Compare November 8, 2022 11:08
@felladrin felladrin marked this pull request as ready for review November 8, 2022 11:11
@felladrin felladrin requested a review from a team November 8, 2022 11:11
@felladrin felladrin changed the title [JetBrains EAP] Improve port forwarding [JetBrains EAP] Port Forwarding improvements Nov 8, 2022
@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch 2 times, most recently from 0399bbb to 5a3b0fe Compare November 8, 2022 12:56
@andreafalzetti
Copy link
Contributor

andreafalzetti commented Nov 9, 2022

/werft run recreate-preview

👍 started the job as gitpod-build-felladrin-improve-port-forwarding.19
(with .werft/ from main)

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Nov 10, 2022

I am reviewing this now 👀

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Nov 10, 2022

I've started reviewing but not finished writing my feedback, I need some more time 🙏

@andreafalzetti
Copy link
Contributor

I'd suggest to hold it / move to draft while we understand more about the "default lock icon".

@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch from 5a3b0fe to 06621da Compare November 11, 2022 09:15
@felladrin felladrin force-pushed the felladrin/improve-port-forwarding branch from 06621da to ac861b2 Compare November 11, 2022 09:16
@felladrin
Copy link
Contributor Author

Icons updated:

image

But it's important to mention that this PR is not about the Ports View, it's about avoiding querying the Supervisor about the status of the ports by syncing the state with the PerClientPortForwardingManager of JetBrains and querying it instead.

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Nov 11, 2022

Icons updated:

image

But it's important to mention that this PR is not about the Ports View, it's about avoiding querying the Supervisor about the status of the ports by syncing the state with the PerClientPortForwardingManager of JetBrains and querying it instead.

Good idea decoupling the icons not to hold the PR 🙏

Code changes LGTM, tested and works as expected 🙏

Nice clean up and improvement 🚀

@roboquat roboquat merged commit 3bf9dd3 into main Nov 11, 2022
@roboquat roboquat deleted the felladrin/improve-port-forwarding branch November 11, 2022 10:00
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production editor: jetbrains release-note-none size/XXL team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants