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

Add hubble observe flows #851

Merged
merged 1 commit into from
Jan 17, 2023
Merged

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Jan 13, 2023

Hubble observe is now an alias for hubble observe flows. The only major difference is the help text for hubble observe is now briefer and indicates it is an alias for hubble observe flows. Additionally, while refactoring, I removed the flow formatting flags from the general formatting flagset and just moved it to it's own flagset.

Fixes #543

@chancez chancez requested a review from a team as a code owner January 13, 2023 20:17
@chancez chancez requested review from tklauser and removed request for a team January 13, 2023 20:17
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label PR is blocked until the release note is set label Jan 13, 2023
@chancez chancez marked this pull request as draft January 13, 2023 20:19
@chancez chancez force-pushed the pr/chancez/hubble_observe_alias branch 2 times, most recently from fe137cd to 9d05514 Compare January 13, 2023 20:52
Hubble observe is now an alias for `hubble observe flows`.
The only major difference is the help text for `hubble observe` is now
briefer and indicates it is an alias for `hubble observe flows`.
Additionally, while refactoring, I removed the flow formatting flags
from the general formatting flagset and just moved it to it's own
flagset.

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
@chancez chancez force-pushed the pr/chancez/hubble_observe_alias branch from 9d05514 to cf7f8e4 Compare January 14, 2023 00:24
@chancez chancez marked this pull request as ready for review January 14, 2023 00:43
@kaworu kaworu added ⌨️ area/cli Impacts the command line interface of any command in the repository. 🌟 kind/feature This introduces new functionality. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble. labels Jan 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label PR is blocked until the release note is set labels Jan 16, 2023
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Thanks a lot for tackling this @chancez! 🙏 Tested and the patch LGTM to me as well.

One funny thing I noticed is that hubble observe flows already works before this patch (I guess the missing subcommand invoke the parent because hubble observe foo also show flows).

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Thanks a lot for picking this up! 🚀

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 17, 2023
@tklauser tklauser merged commit fd73f31 into master Jan 17, 2023
@tklauser tklauser deleted the pr/chancez/hubble_observe_alias branch January 17, 2023 14:16
@gandro
Copy link
Member

gandro commented Jan 24, 2023

@chancez I think this PR broke the shorthand flags:

./hubble observe -o json
unknown shorthand flag: 'o' in -o

Only occurs on master, v0.11 is fine.

@rolinh
Copy link
Member

rolinh commented Jan 24, 2023

I think it's not just shorthand flags; formatting flags seem to be broken:

$ hubble observe --output json
unknown flag: --output
$ hubble observe flows --output json
unknown flag: --output

EDIT: selector flags as well it seems

$ hubble observe flows --follow
unknown flag: --follow

@chancez
Copy link
Contributor Author

chancez commented Jan 24, 2023

@rolinh Ah. I guess tests and my manual usage weren't using these flags. Let's revert and I can try again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌨️ area/cli Impacts the command line interface of any command in the repository. 🌟 kind/feature This introduces new functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hubble observe flows as alias for hubble observe
5 participants