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

[gp-cli] Add alias for forward and await in ports #10538

Merged
merged 2 commits into from
Jun 13, 2022
Merged

[gp-cli] Add alias for forward and await in ports #10538

merged 2 commits into from
Jun 13, 2022

Conversation

CuriousCorrelation
Copy link
Contributor

Description

This PR aliases forward-port and await-port command to ports namespace as ports forward, ports await

Related Issue(s)

Closes #10449

How to test

Both commands should be similar except for their Usage message

forward-port

❯ gp forward-port --help
Makes a port available on 0.0.0.0 so that it can be exposed to the internet

Usage:
  gp forward-port <local-port> [target-port] [flags]

Flags:
  -h, --help                  help for forward-port
  -r, --rewrite-host-header   rewrites the host header of passing HTTP requests to localhost
❯ gp ports forward --help
Makes a port available on 0.0.0.0 so that it can be exposed to the internet

Usage:
  gp ports forward <local-port> [target-port] [flags]

Flags:
  -h, --help                  help for forward
  -r, --rewrite-host-header   rewrites the host header of passing HTTP requests to localhost

await-port

❯ gp await-port --help
Waits for a process to listen on a port

Usage:
  gp await-port <port> [flags]

Flags:
  -h, --help   help for await-port
❯ gp ports await --help
Waits for a process to listen on a port

Usage:
  gp ports await <port> [flags]

Flags:
  -h, --help   help for await

Release Notes

Added alias for forward and await in ports CLI namespace

Documentation

Does this PR require updates to the documentation at www.gitpod.io/docs?

  • Yes

https://github.com/gitpod-io/website/issues/2173#issue-1260679472

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 8, 2022

/werft run

👍 started the job as gitpod-build-main-fork.39
(with .werft/ from main)

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 8, 2022

@CuriousCorrelation The build is failing because of a linting issue with Go. Could you run gofmt -w . from components/gitpod-cli? 🙏

Remember to squash your next commit, you can use: git commit --amend --no-edit and git push origin HEAD -f :)

@mustard-mh
Copy link
Contributor

One suggestion is, can we switch the alias order, i.e. await <port> should be real command, await-port should be alias.

@iQQBot
Copy link
Contributor

iQQBot commented Jun 9, 2022

One suggestion is, can we switch the alias order, i.e. await <port> should be real command, await-port should be alias.

I think it's a better solution

@akosyakov
Copy link
Member

Do we really need ports forward? Don't we proxy all ports by now by default already? I would like to keep forwarding for other means, like remote/local forwarding.

@roboquat roboquat added size/M and removed size/S labels Jun 9, 2022
@CuriousCorrelation
Copy link
Contributor Author

CuriousCorrelation commented Jun 9, 2022

@andreafalzetti I bookmarked wrong pipeline and thought "everything looks good 🤓, must be something else...". Really sorry for the confusion! It should be fixed now.

@CuriousCorrelation
Copy link
Contributor Author

I'd be happy to work on changing the ergonomics once there's a consensus.

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 9, 2022

Do we really need ports forward? Don't we proxy all ports by now by default already? I would like to keep forwarding for other means, like remote/local forwarding.

How can we name it? gp ports bind?

One suggestion is, can we switch the alias order, i.e. await should be real command, await-port should be alias.

Agree - we could always iterate and keep contributions flowing IMHO, but I also appriciate that if we don't track it, we won't do it in the future, so let's do it in this PR 👍

@akosyakov
Copy link
Member

How can we name it? gp ports bind?

I ten to call it expose. But asked internally https://gitpod.slack.com/archives/C01KGM9DVRC/p1654844724560089

@andreafalzetti
Copy link
Contributor

For visibility: Internal consensus was expose

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 10, 2022

It seems that agreed changes, are:

  • Move the commands code/logic and semantic closer to the ports file/cmd as suggested by @mustard-mh [1]
  • Alias the previous cmd instead
  • The new command will be gp ports expose instead of forward
  • We also discussed that it would be useful to add a deprecation warning on the old cmd.

What do you think, @CuriousCorrelation? Is this something you feel confortable moving forward?

@CuriousCorrelation
Copy link
Contributor Author

CuriousCorrelation commented Jun 10, 2022

@andreafalzetti These changes do seem better ergonomically and I'd love to work on them. I'll let you know if I have any questions or if there are any major changes.

Just to be extra sure, the new commands will be

gp ports expose <port>

and

gp ports await <port>

each with aliases

gp await-port <port>

and

gp forward-port <port>

respectively (hidden with deprecation warning).

Does that sound alright?

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 10, 2022

That sounds right @CuriousCorrelation! 🧡

`await-port` is now deprecated. New command is `gp ports await`.
`await-port` is now an alias for backwards compatibility.
`forward-port` is now deprecated. New cmd is `gp ports expose`.
`forward-port` is now an alias for `ports expose`
for backwards compatibility.
@CuriousCorrelation
Copy link
Contributor Author

CuriousCorrelation commented Jun 11, 2022

Latest commit is up and repo is updated.

These are the changes


Commands

❯ gp --help
Command line interface for Gitpod

Usage:
  gp [command]

Available Commands:
  env                 Controls user-defined, persistent environment variables.
  help                Help about any command
  init                Create a Gitpod configuration for this project.
  open                Opens a file in Gitpod
  ports               Interact with workspace ports.
  preview             Opens a URL in the IDE's preview
  snapshot            Take a snapshot of the current workspace
  stop                Stop current workspace
  sync-await          Awaits an event triggered using gp sync-done
  sync-done           Notifies the corresponding gp sync-await calls that this event has happened
  tasks               Interact with workspace tasks
  url                 Prints the URL of this workspace
  version             Prints the version of the CLI

Flags:
  -h, --help   help for gp

Use "gp [command] --help" for more information about a command.

Ports now mentions expose instead of forward

❯ gp ports --help
Interact with workspace ports.

Usage:
  gp ports [flags]
  gp ports [command]

Available Commands:
  await       Waits for a process to listen on a port
  expose      Makes a port available on 0.0.0.0 so that it can be exposed to the internet
  list        Lists the workspace ports and their states.

Flags:
  -h, --help   help for ports

Use "gp ports [command] --help" for more information about a command.

forward-port now emits a deprecation warning.

❯ gp forward-port --help
Command "forward-port" is deprecated, please use `ports expose` instead.
Makes a port available on 0.0.0.0 so that it can be exposed to the internet

Usage:
  gp forward-port <local-port> [target-port] [flags]

Flags:
  -h, --help                  help for forward-port
  -r, --rewrite-host-header   rewrites the host header of passing HTTP requests to localhost

forward-port now moved to ports expose

❯ gp ports expose --help
Makes a port available on 0.0.0.0 so that it can be exposed to the internet

Usage:
  gp ports expose <local-port> [target-port] [flags]

Flags:
  -h, --help                  help for expose
  -r, --rewrite-host-header   rewrites the host header of passing HTTP requests to localhost

await-port now emits deprecation warning

❯ gp await-port --help
Command "await-port" is deprecated, please use `ports await` instead.
Waits for a process to listen on a port

Usage:
  gp await-port <port> [flags]

Flags:
  -h, --help   help for await-port

ports await is now the main command

❯ gp ports await --help
Waits for a process to listen on a port

Usage:
  gp ports await <port> [flags]

Flags:
  -h, --help   help for await

Files

Now that ports is a plural command like tasks

components/gitpod-cli/cmd/forward-port.go and components/gitpod-cli/cmd/await-port.go

are moved to

components/gitpod-cli/cmd/ports-expose.go and components/gitpod-cli/cmd/ports-await.go

respectively, modeled after tasks

components/gitpod-cli/cmd/tasks-list.go and components/gitpod-cli/cmd/tasks-attach.go

Commits

Divided into two commits, one for changes to await and one for forward for easier tracking. Commit message body mentions deprecation.


@andreafalzetti Let me know what you think about this and if I missed something!

@loujaybee
Copy link
Member

loujaybee commented Jun 13, 2022

Just dropping by to offer a big thank you for making this contribution @CuriousCorrelation 🙏

It's looking great, and thank you for responding to feedback so quickly! 🚀

@CuriousCorrelation
Copy link
Contributor Author

@loujaybee My pleasure. Thank you and the team for maintaining an excellent service that I love to use daily!

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 13, 2022

/werft run

👍 started the job as gitpod-build-main-fork.40
(with .werft/ from main)

Copy link
Contributor

@andreafalzetti andreafalzetti left a comment

Choose a reason for hiding this comment

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

Changes look great, thanks a lot @CuriousCorrelation 🙏

We really appreciate your contribution 🙌

We will deploy this soon (I hope tomorrow) :)

@roboquat roboquat merged commit 311d13c into gitpod-io:main Jun 13, 2022
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Jun 14, 2022
@andreafalzetti
Copy link
Contributor

andreafalzetti commented Jun 14, 2022

@CuriousCorrelation I've released this PR to prod 🚀

CuriousCorrelation added a commit to CuriousCorrelation/website that referenced this pull request Jun 14, 2022
gitpod-io/gitpod#10538 PR
moved `await-port` functionality to `ports await`
and `forward-port`, now renamed `expose`,
moved to `ports expose`

Previously:
- `gp forward-port <port>`
- `gp await-port <port>`

Now:
- `gp ports expose <port>`
- `gp ports await <port>`
CuriousCorrelation added a commit to CuriousCorrelation/website that referenced this pull request Jun 14, 2022
gitpod-io/gitpod#10538 PR
moved `await-port` functionality to `ports await`
and `forward-port`, now renamed `expose`,
moved to `ports expose`

Previously:
- `gp forward-port <port>`
- `gp await-port <port>`

Now:
- `gp ports expose <port>`
- `gp ports await <port>`
CuriousCorrelation added a commit to CuriousCorrelation/website that referenced this pull request Jun 22, 2022
gitpod-io/gitpod#10538 PR
moved `await-port` functionality to `ports await`
and `forward-port`, now renamed `expose`,
moved to `ports expose`

Previously:
- `gp forward-port <port>`
- `gp await-port <port>`

Now:
- `gp ports expose <port>`
- `gp ports await <port>`
tnir added a commit to tnir/workspace-images that referenced this pull request Jul 22, 2022
sagor999 pushed a commit to gitpod-io/workspace-images that referenced this pull request Jul 22, 2022
axonasif pushed a commit to gitpod-io/workspace-images that referenced this pull request Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: accepted ✔️ community-contribution deployed: IDE IDE change is running in production deployed Change is completely running in production release-note size/M team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move all ports commands under the gp CLI namespace
7 participants