-
Notifications
You must be signed in to change notification settings - Fork 373
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(gnovm): don't print to stdout by default #3076
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3076 +/- ##
==========================================
- Coverage 63.33% 63.32% -0.01%
==========================================
Files 548 548
Lines 78646 78652 +6
==========================================
+ Hits 49807 49809 +2
- Misses 25478 25479 +1
- Partials 3361 3364 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
By default or with a flag? |
@gfanton what about gnodev? |
By default. I think this shouldn't be configurable on the gnoland node per se?
This was also a concern of mine, but I was eager to get a fix out asap as from what I understood this was a show-stopper bug, but seems like it wasn't too much after all. I went back and added an option so that gnodev can set os.Stdout as the output. |
I agree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can keep gnodev
this way for now; it's partially fine. Just remember that gnodev
operates in raw term mode, which means newlines should be written as \r\n
. As a result, outputting directly to stdout will not display as expected. These logs are infrequent on gnodev
during local development, so it's manageable, but it remains a temporary solution.
This issue is part of a larger problem: gnodev
currently cannot display node logs. We need to implement a proper method for displaying both node logs and this type of output without affecting the readability of gnodev
logs. It's important to note that node logs are rarely needed for a lambda user, as they tend to be excessively verbose.
I think we can simply restrict it to errors, so avoiding all the debug messages; and then maybe these ones printed to stdout. We can make it work also by putting, as the output, a Writer that replaces all Anyhow, this is for you to figure out in gnoweb if you have the time. Merging for now. |
Attempt to fix #3075
cc/ @zivkovicmilos @sw360cab