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

Use Log output instead of standard out #62

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

stefaniereuter
Copy link
Contributor

this is a suggestion to avoid spamming the output in runs. With many IO servers this can sometimes become a long list.

@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 53.06%. Comparing base (8712a46) to head (34cfb7b).

Files with missing lines Patch % Lines
src/multio/config/MultioConfiguration.cc 0.00% 5 Missing ⚠️
src/multio/ifsio/ifsio.cc 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop      #62   +/-   ##
========================================
  Coverage    53.06%   53.06%           
========================================
  Files          217      217           
  Lines        14644    14644           
  Branches      1213     1213           
========================================
  Hits          7771     7771           
  Misses        6873     6873           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tweska
Copy link
Member

tweska commented Mar 12, 2025

I wasn't aware of this macro, I used eckit::Log::info() and eckit::Log::warning() before. We should agree on something and have an internal FAQ with things like: "How to log/print", "How to throw an exception", etc...

@stefaniereuter
Copy link
Contributor Author

@tweska As far as I know @MircoValentiniECMWF also had longer discussions with @geier1993 and @dsarmany of having multiple MULTIO flags to enable debug on specific code regions. e.g. Multio_statistic ...

Copy link
Collaborator

@dsarmany dsarmany left a comment

Choose a reason for hiding this comment

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

I think we still want to print these also in no-debug runs.

@@ -61,14 +62,14 @@ ConfigAndPaths configureFromEnv(config::LocalPeerTag tag) {

if (::getenv("MULTIO_PLANS")) {
std::string cfg(::getenv("MULTIO_PLANS"));
std::cout << "MultIO initialising with plans " << cfg << std::endl;
LOG_DEBUG_LIB(LibMultio) << "MultIO initialising with plans " << cfg << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may want to use eckit::Log::info(), no? I think there is no harm printing this all the time as the output is fairly limited.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should consider printing this only from one server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of only printing it from one server. This would be another reason to have this IO-Server "Master".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You may want to use eckit::Log::info(), no? I think there is no harm printing this all the time as the output is fairly limited.

Well for smaller runs it is limited but if they run larger production runs it does get more annoying. The information in general is useful, we probably just don't want to have it printed so often.

Essentially this is a non urgent, "make it look prettier", idea, to reduce log messages that can't be turned off. The output scripts are annoying to search through as it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, I can just merge this and see how much it is missed. Then we can come up with a cleverer solution.

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

Successfully merging this pull request may close these issues.

5 participants