ESMValGroup / ESMValCore

ESMValCore: A community tool for pre-processing data from Earth system models in CMIP and running analysis scripts.
https://www.esmvaltool.org
Apache License 2.0
42 stars 38 forks source link

Log CMOR check and generic fix output to separate file #2361

Closed schlunma closed 4 months ago

schlunma commented 6 months ago

Description

This PR adds filters to our logging configuration so that output from our CMOR checks and generic fixes will not appear in stdout and main_log.txt anymore, but rather in a dedicated file cmor_log.txt. This makes the output of the tool much cleaner.

Closes #2351


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the πŸ›  Technical or πŸ§ͺ Scientific review.


To help with the number pull requests:

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.30%. Comparing base (bbd307d) to head (575129b). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2361 +/- ## ======================================= Coverage 94.29% 94.30% ======================================= Files 246 246 Lines 13540 13559 +19 ======================================= + Hits 12768 12787 +19 Misses 772 772 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

schlunma commented 6 months ago

Questions I have before proceeding with this:

  1. Should main_log_debug.txt still contain everything, i.e., also the CMOR-related messages? I think it should.
  2. Should the cmor_log.txt also include CMOR-related debug messages or only warnings? I think it should only include warnings.

@ESMValGroup/technical-lead-development-team

valeriupredoi commented 5 months ago

Should the cmor_log.txt also include CMOR-related debug messages or only warnings? I think it should only include warnings.

I reckon only warnings is enough - we don't want to fix CMOR, we want to fix our own CMOR-related issues :+1:

Cheers for doing this, bud! Lemme know when you ready for a revue :fr:

schlunma commented 5 months ago

Cheers for doing this, bud! Lemme know when you ready for a revue πŸ‡«πŸ‡·

It is now 😁

schlunma commented 5 months ago

many thanks, Manu! If I understand correctly, we decided the CMOR log gets outputted always, not only when the main tool is run in logging DEBUG, right?

That DEBUG here means that all messages (debug and above) are logged to that file. It will always be written, regardless of the user's log settings.

This directly relates to my question above:

  1. Should main_log_debug.txt still contain everything, i.e., also the CMOR-related messages? I think it should.

If I set this to WARNING, then only warning messages and above are logged. Is this what we want? I guess yes? Note: the debug messages will always be in the main_log_debug.txt file.

valeriupredoi commented 5 months ago

That DEBUG here means that all messages (debug and above) are logged to that file. It will always be written, regardless of the user's log settings.

Gotcha! Good call! :beers:

And yes, I'd argue only CMOR WARNING messages be written to it - but what do others think about it, am looking at @sloosvel @ehogan and @bouweandela specifically :beer:

ehogan commented 5 months ago

That DEBUG here means that all messages (debug and above) are logged to that file. It will always be written, regardless of the user's log settings.

Gotcha! Good call! 🍻

And yes, I'd argue only CMOR WARNING messages be written to it - but what do others think about it, am looking at @sloosvel @ehogan and @bouweandela specifically 🍺

Just to check my understanding: it will be possible to (always) see all CMOR logs via main_log_debug.txt, but only warnings will be shown via cmor_log.txt and main_log.txt? If so, that seems reasonable to me! πŸ‘

schlunma commented 5 months ago

Just to check my understanding: it will be possible to (always) see all CMOR logs via main_log_debug.txt, but only warnings will be shown via cmor_log.txt and main_log.txt? If so, that seems reasonable to me! πŸ‘

CMOR warnings would not be shown in main_log.txt (only in cmor_log.txt), apart from that that's correct!

valeriupredoi commented 4 months ago

okidoki if there are no more suggestions then I think we should be go here :+1:

valeriupredoi commented 4 months ago

@bouweandela would you mind doing a final sweep and merge if all's good in yer books, plss, bud :beer: