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.

Sign up for GitHub

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

Jump to bottom

Revamp logger module for additional features and bug fixes #924

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Revamp logger module for additional features and bug fixes #924

wants to merge 3 commits into from

Conversation

tomvothecoder
Copy link
Collaborator

@tomvothecoder tomvothecoder commented Jan 22, 2025 •
edited
Loading

Description

Task 1 - Save log file directly to prov dir initially instead of moving at the end

Task 2 - Add feature to print diagnostic run information at the top

  • Readable formatting
  • Content includes timestamp, git branch and commit or version info, log filepath

Task 3 - Fixes to the logger

  • Fix issue where logger is not saving the complete output from the console to the log file (Closes [Bug]: prov log doesn't include complete traceback #909)
  • Fix issue where warnings are not being shown in the console but show in the log file -- IN PROGRESS
  • Make sure console and log file outputs are consistent -- IN PROGRESS
    • Both should have a log filemode of w
  • Testing
    • Works with serial
    • Works with multiprocessing
    • Works with multiple simulations in a for-loop

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

@tomvothecoder tomvothecoder self-assigned this Jan 22, 2025
@tomvothecoder tomvothecoder added the bug Bug fix (will increment patch version) label Jan 22, 2025
@chengzhuzhang
Copy link
Contributor

@tomvothecoder Another thing that bothers me a little, is that the e3sm_diags_run.log file was created in the directory where the run script is located, I'm wondering if it makes more sense to have it initiate under /prov, right away?

@tomvothecoder tomvothecoder marked this pull request as draft January 22, 2025 23:24
@tomvothecoder tomvothecoder changed the title Create one logger object per instantiated Run() object Create one logger object per instantiated Run() object and fix logger not including full traceback Jan 23, 2025
@tomvothecoder
Copy link
Collaborator Author

@tomvothecoder Another thing that bothers me a little, is that the e3sm_diags_run.log file was created in the directory where the run script is located, I'm wondering if it makes more sense to have it initiate under /prov, right away?

I agree, it makes sense to store the log immediately in the /prov rather than wait for a diagnostic run to end. There is also a semi-frustrating annoying behavior where when diagnostic run ends early (either manually or when crashing), the log won't be moved to /prov.

To achieve this, we need to find a way to pass the /prov dir to the logger object prior to any logging.

@tomvothecoder
Add new features to logger and fix bugs
41a7a71 
- Saves log file directly to `prov` dir
- Fix issue where complete log output not being printed to logfile
- Add feature to print diagnostic run information at the top of the logger output
@tomvothecoder tomvothecoder changed the title Create one logger object per instantiated Run() object and fix logger not including full traceback Revamp logger module for additional features and bug fixes Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@tomvothecoder tomvothecoder

Labels
bug Bug fix (will increment patch version)
Projects
None yet
Milestone
FY25 Q2 (01/01/25 - 03/31/25)
Development

Successfully merging this pull request may close these issues.

[Bug]: prov log doesn't include complete traceback [Bug]: e3sm_diags_run.log was not properly generated
2 participants
@tomvothecoder @chengzhuzhang