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

fix(iroh-cli): Fix printing of doctor connect/accept output #2166

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

flub
Copy link
Contributor

@flub flub commented Apr 10, 2024

Description

Apparently now metrics work and are attempted to be printed.
Unfortunately they also print a tracing error message to stderr, the
stderr which is also being used by the progress bar. The result is
that the progress bar ends up with an extra newline and the first line
of it is not being repainted but flows up, resulting in lots of
garbage in the terminal history.

By removing the log message the output is perserved much better.

Notes & open questions

It could be argued that this is not a good fix, if the loglevel is
changed to info or debug the same problems would occur. However we
already have the ability to log to a random filedescriptor and will
soon be able to log to files. So maybe that's just fine for now.

Change checklist

  • Self-review.
  • [ ] Documentation updates if relevant.
  • [ ] Tests if relevant.

Apparently now metrics work and are attempted to be printed.
Unfortunately they also print a tracing error message to stderr, the
stderr which is also being used by the progress bar.  The result is
that the progress bar ends up with an extra newline and the first line
of it is not being repainted but flows up, resulting in lots of
garbage in the terminal history.

By removing the log message the output is perserved much better.

It could be argued that this is not a good fix, if the loglevel is
changed to info or debug the same problems would occur.  However we
already have the ability to log to a random filedescriptor and will
soon be able to log to files.  So maybe that's just fine for now.
@flub flub requested a review from rklaehn April 10, 2024 11:10
@dignifiedquire dignifiedquire added this to the v0.14.0 milestone Apr 10, 2024
@flub flub added this pull request to the merge queue Apr 10, 2024
Merged via the queue into main with commit 5d4ac52 Apr 10, 2024
20 checks passed
@flub flub deleted the flub/doctor-pb-newlines branch April 10, 2024 15:15
ppodolsky pushed a commit to izihawa/iroh that referenced this pull request Apr 11, 2024
…ter#2166)

## Description

Apparently now metrics work and are attempted to be printed.
Unfortunately they also print a tracing error message to stderr, the
stderr which is also being used by the progress bar.  The result is
that the progress bar ends up with an extra newline and the first line
of it is not being repainted but flows up, resulting in lots of
garbage in the terminal history.

By removing the log message the output is perserved much better.

## Notes & open questions

It could be argued that this is not a good fix, if the loglevel is
changed to info or debug the same problems would occur.  However we
already have the ability to log to a random filedescriptor and will
soon be able to log to files.  So maybe that's just fine for now.

## Change checklist

- [x] Self-review.
- ~~[ ] Documentation updates if relevant.~~
- ~~[ ] Tests if relevant.~~
ppodolsky pushed a commit to izihawa/iroh that referenced this pull request Apr 11, 2024
…ter#2166)

## Description

Apparently now metrics work and are attempted to be printed.
Unfortunately they also print a tracing error message to stderr, the
stderr which is also being used by the progress bar.  The result is
that the progress bar ends up with an extra newline and the first line
of it is not being repainted but flows up, resulting in lots of
garbage in the terminal history.

By removing the log message the output is perserved much better.

## Notes & open questions

It could be argued that this is not a good fix, if the loglevel is
changed to info or debug the same problems would occur.  However we
already have the ability to log to a random filedescriptor and will
soon be able to log to files.  So maybe that's just fine for now.

## Change checklist

- [x] Self-review.
- ~~[ ] Documentation updates if relevant.~~
- ~~[ ] Tests if relevant.~~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants