fix: Output writing #804
Merged
fix: Output writing #804
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This simply delegates to eprintln! for writing output. This ensures writes happen more synchronously and the flaky integration tests are fixed. While in theory these are blocking operations on the async executor, this is acceptable because: - We already use println!() without any problems. Arguably this is more of an issue since we also write data to stdout but it isn't because we don't do that at the same time. - We print a small number of messages, not large amounts of data. Any blocking from this will be small. - This acquires a sync mutex while writing, that's fine because it is acquired and released in the same chunk of async code and not held across await points. This is unlikely to block much as only main writes and only one write happens at a time. - When logging via tracing is enabled this will also end up logging on here and will compete for the lock. This is slightly higher contention but doesn't really interfere with the logging here.
|
I'll wait for approval by @ramfox as well since I inadvertently stepped on their PR. |
This was referenced Feb 28, 2023
| // Print the formatted string to the console with a newline | ||
| message.push('\n'); | ||
| tokio::io::stderr().write_all(message.as_ref()).await.unwrap(); | ||
| eprintln!($fmt $(, $args)*); |
There was a problem hiding this comment.
Works for me, until this becomes an issue, lets just do this
|
🔥 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This simply delegates to eprintln! for writing output. This ensures
writes happen more synchronously and the flaky integration tests are
fixed.
While in theory these are blocking operations on the async executor,
this is acceptable because:
We already use println!() without any problems. Arguably this is
more of an issue since we also write data to stdout but it isn't
because we don't do that at the same time.
We print a small number of messages, not large amounts of data. Any
blocking from this will be small.
This acquires a sync mutex while writing, that's fine because it is
acquired and released in the same chunk of async code and not held
across await points. This is unlikely to block much as only main
writes and only one write happens at a time.
When logging via tracing is enabled this will also end up logging on
here and will compete for the lock. This is slightly higher
contention but doesn't really interfere with the logging here.