-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix Supervisor sending empty port names and descriptions #9797
Conversation
0dc177d
to
34df117
Compare
The current code is working, as seen in here. What is wrong is that it's not reading the the .gitpod.yml file as it should. |
34df117
to
98726d3
Compare
@mustard-mh, just to let you know I'll add more details to the PR description soon, to make it clear what's the problem and what I have already tried to fix it. Hopefully you can help to find the cause ^^ |
74fa049
to
98c03ec
Compare
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.
LGTM Thanks @mustard-mh
98c03ec
to
073b8e0
Compare
Does it work for port ranges too? |
073b8e0
to
3013055
Compare
I think it will after the new push, waiting for new build to test 👍 |
Ranges will work only when ports are opened. cc @akosyakov |
Hm, there is no name in the remote explorer view. It is a bug in vscode web. Could you fix it there as well? 🙏 |
What
|
but it is another follow up PR, this one is good 👍 |
@@ -411,6 +411,11 @@ func (pm *Manager) nextState(ctx context.Context) map[uint32]*managedPort { | |||
|
|||
var public bool | |||
config, kind, exists := pm.configs.Get(mp.LocalhostPort) | |||
if exists { |
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.
I wonder why it is necessary here, configurations should be handled above at step 2, this step about served state
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.
ah, it is for ranges?
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.
Do we miss here anything else like visibility handling or action on expose?
It can be another PR too.
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.
visibility (port private and public) is handled here
mp.AutoExposure = pm.autoExpose(ctx, mp.LocalhostPort, public).state |
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.
Do we need to call mp.OnExposed = getOnExposedAction(config, port)
? or it is handled already somewhere too?
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.
Do we need to call mp.OnExposed = getOnExposedAction(config, port)? or it is handled already somewhere too?
added
I tested with onOpen: open-browser
with ranges before and it works fine, but I don't know where is code. Add to confirm it
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.
thank you, great 👍
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.
maybe a commit in if will be nice, that we are handling configuration for port ranges here
3013055
to
cf43f79
Compare
Description
Currently, supervisor send empty name and description of ports to editor(code) which make tooltip in code not works (see also #9262 (comment)).
This PR fix it
Related Issue(s)
#9262 (comment)
How to test
Note that port with range will work only when ports are opened
Init with .gitpod.yml
.gitpod.yml
with name and descripts like this one https://github.com/mustard-mh/test/tree/hw/port-nameWatch .gitpod.yml
.gitpod.yml
in your workspace, the tooltip inport explorer
should reflect toyml
fileRelease Notes
Documentation