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

When resolving to stdout do not display status #184

Open
MadVikingGod opened this issue May 29, 2024 · 6 comments
Open

When resolving to stdout do not display status #184

MadVikingGod opened this issue May 29, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@MadVikingGod
Copy link
Contributor

Describe the bug
When the output is stdout, the status lines (example below) will break any kind of stream processing, such as passing it to jq.

example:

✔ SemConv registry loaded (111 files)
✔ No policy found
✔ SemConv registry resolved

To Reproduce
Steps to reproduce the behavior:

  1. Run weaver registry resolve -f json | jq .
  2. watch it fail because of the status printed at the beginning

Expected behavior
The resolve output should be in the correct format, free of any status outputs.

Additional context
These statuses are nice if we are outputting to a file or if they were on stderr.

@lquerel
Copy link
Contributor

lquerel commented May 29, 2024

It's not ideal but weaver --quiet registry resolve -f json | jq . works.

However, I agree we should turn automatically quiet mode in this specific context. Thanks for reporting.

@lquerel lquerel added enhancement New feature or request good first issue Good for newcomers labels May 29, 2024
@lquerel lquerel removed the good first issue Good for newcomers label May 29, 2024
@lquerel
Copy link
Contributor

lquerel commented May 30, 2024

--quiet is no longer required with #187 (not yet approved at this point of time).

@MadVikingGod
Copy link
Contributor Author

Is there any way to promote this option to A) show up in the weaver registry resolve --help, and B) Be usable from the end of the command?

For B) weaver --quiet registry resolve works, but weaver registry resolve --quiet does not.

@lquerel
Copy link
Contributor

lquerel commented May 31, 2024

Yes, it's definitely something to improve in the user experience. We use Clap for command-line parsing, and while there are some limitations, there are workarounds to ensure that both A and B are feasible.

@lquerel
Copy link
Contributor

lquerel commented Jun 3, 2024

@MadVikingGod with #187 merged --quiet is no longer required.

However, I'd like to keep this issue open as I will make some changes to accept the --quiet argument in a more discoverable position.

@lquerel
Copy link
Contributor

lquerel commented Jun 5, 2024

Fixed in #196 (pending approval).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants