-
Notifications
You must be signed in to change notification settings - Fork 111
Conversation
This is very cool ! |
@benoitf gifs look great! could you add a bit of detail on how this is implemented? Thanks! |
@garagatyi you looked at the README ? |
Oh, I missed the very first line of the description. Sorry |
- if there is no workspace server matching, propose the user to do a port redirect. | ||
|
||
|
||
Port redirect mechanism: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means that some ingress or routes are pre-created when starting the workspace? That's a smart way to solve the problem though :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@l0rd yes correct. There is a need of a 'pool' of routes that can be later used when need to proxify a port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really cool
If a process is listening, it will check | ||
- if listening interface is 127.0.0.1/::1 it asks the user to create a redirect for this port so it will externally available | ||
- if listening remotely: | ||
- if there is a workspace server that is matching for this port, display the URL of this server and propose to open the link. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to talk about pre-exposed endpoint
rather than workspace server
? And display the URL of this endpoint
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@l0rd well in that case it's not really a pre-exposed endpoint
For example a user has specified in its workspace configuration a server, let say 'express' with port 3000
in that case we just show the public URL when the port is listening.
But of course I can change the wording. But this bullet was referencing the case of a valid workspace that is exposing ports as expected. In that case the plug-in does only 'show public URL'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it's not pre-exposed
I had misunderstood. But anyway it's the workspace server
that is a Che 6 terminology. In the devfile and che-plugin.yaml we will call it endpoint I think so we may use the new term here as well.
Ready to review :-) |
I'm not appropriate person to add my review to this PR, but I'm +1 this solution very much - great idea of overcoming our contraints! |
maybe it would be better to change popup buttons to "yes" / "no" for consistency |
@ibuziuk yes sure thx for the feedback |
095921f
to
08f306c
Compare
Change-Id: Iaa91b1d2c8c31e8ebd0047c9bf53e8baa76c888e Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Change-Id: Ic6fc89176353c951320502e5267645faf4e4284f Signed-off-by: Florent Benoit <fbenoit@redhat.com>
54c1b25
to
fafe221
Compare
Description in README file:
https://github.com/eclipse/che-theia/blob/095921fd96c3481db28e0374524bef604e3d6f67/plugins/ports-plugin/README.md
example of a process starting to listen on a port
example of a process listening on localhost/127.0.0.1
note: should be smoother with eclipse-theia/theia#4243
Change-Id: Iaa91b1d2c8c31e8ebd0047c9bf53e8baa76c888e
Signed-off-by: Florent Benoit fbenoit@redhat.com