Do not log self and do not write ANSI color codes when writing/piping output to a file#73
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #73 +/- ##
===========================================
+ Coverage 60.56% 60.72% +0.15%
===========================================
Files 24 24
Lines 1973 1986 +13
Branches 370 374 +4
===========================================
+ Hits 1195 1206 +11
- Misses 724 726 +2
Partials 54 54 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…Y streams Agent-Logs-Url: https://github.com/NOAA-EMC/wxflow/sessions/94ef3770-3e22-4a0d-b5f2-2e1a92fea70d Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
…dd_stream_logger Agent-Logs-Url: https://github.com/NOAA-EMC/wxflow/sessions/a11632c0-b4f0-467d-bf41-56329914d73b Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
|
Merged #74 into this PR. This removes ANSI formatting characters when piping output to a log file. |
selfself and do not write ANSI color codes when writing/piping output to a file
There was a problem hiding this comment.
Pull request overview
This PR updates wxflow’s logging utilities to (1) suppress ANSI color escape codes when output is redirected/piped to non-TTY streams and (2) adjust the logit decorator’s argument logging for instance methods.
Changes:
- Extend
add_stream_loggerto accept an optional stream and only applyColoredFormatterwhen the stream is a TTY. - Modify
logitto avoid logging fullselfrepresentations (currently by substituting a placeholder). - Add tests to detect ANSI escape codes in file / non-TTY streams and exercise
logiton an instance method.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/wxflow/logger.py |
Adds TTY detection for colored formatting and changes logit argument formatting logic. |
tests/test_logger.py |
Adds ANSI escape detection regex and multiple tests for TTY vs non-TTY behavior plus an instance-method logit test. |
Comments suppressed due to low confidence (1)
src/wxflow/logger.py:277
- The changes to
logitsignificantly alter what gets logged for positional args, but the tests don’t assert that normal functions still log their actual argument values (e.g.,add(2, 3)should log2and3, not placeholders). Adding an assertion against the logfile/stream output for a non-method function would prevent regressions in argument logging while still ensuringselfis skipped/obfuscated for instance methods.
# Get all of the arguments passed to the function and log them, skipping 'self'.
passed_args = []
# Determine if the function is an instance method.
if len(args) > 0:
if hasattr(args[0], '__class__'):
class_name = args[0].__class__.__name__
for aa in args:
if isinstance(aa, args[0].__class__):
passed_args.append(f"{class_name} object")
else:
passed_args.append(repr(aa))
else:
passed_args = [repr(aa) for aa in args]
passed_kwargs = [f"{kk}={repr(vv)}" for kk, vv in list(kwargs.items())]
call_msg = 'BEGIN: ' + log_msg
logger.info(call_msg)
logger.debug(f"( {', '.join(passed_args + passed_kwargs)} )")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@TravisElless-NOAA could you review this PR? |
Description
When using the
logitdecorator on an instance method, this will preventselffrom being logged. This improves readability of the logs.Refs NOAA-EMC/global-workflow#4721
Resolves #70
Type of change
How Has This Been Tested?
Ran a global-workflow stage_ic job. This should be tested more thoroughly to ensure NCO approval of this method.
Checklist