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

[WIP] App/Services showing ports subcommand skywire-cli visor ports #1412

Merged
merged 16 commits into from
Dec 8, 2022
Merged

[WIP] App/Services showing ports subcommand skywire-cli visor ports #1412

merged 16 commits into from
Dec 8, 2022

Conversation

mrpalide
Copy link
Contributor

@mrpalide mrpalide commented Nov 12, 2022

Did you run make format && make check? Yes

Fixes #1404

Changes:

  • add new subcommand to skywire-cli as skywire-cli visor ports to list of all involved ports
  • add new methods GetAppPort and SetAppPort to app-cl
  • add flag to show all/running ports, running ports as default and --all flag will show all ports

How to test this PR:

  • run visor
  • run skywire-cli visor ports

Screenshot:
lsof
image

skywire-cli visor ports
image

@0pcom
Copy link
Collaborator

0pcom commented Nov 13, 2022

since we have a way of getting the ports from the visor now, perhaps change everywhere we have used a hardcoded reference to the hypervisor UI such as in the skywire-cli vpn ui & url and skywire-cli hv ui & url to instead fetch this port via the visor rpc. If the port is not found because the visor isnt running, we can just open to or print the default 8000

@0pcom
Copy link
Collaborator

0pcom commented Nov 20, 2022

The ports for apps shown in the config are not the actual ports which are bound to / listened on.

Is it random for these? It just binds to any open port above 1000?

@0pcom 0pcom self-requested a review December 8, 2022 13:58
@0pcom 0pcom merged commit ae552ce into skycoin:develop Dec 8, 2022
@mrpalide mrpalide deleted the feature/app-service-with-port-list-subcommand branch January 8, 2023 16:36
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.

2 participants