-
Notifications
You must be signed in to change notification settings - Fork 97
Reduce log file size - SMT is logging at DEBUG level #944
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
Reduce log file size - SMT is logging at DEBUG level #944
Conversation
81295a9 to
4cede19
Compare
Bischoff
left a comment
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.
The code looks good to me, I tested it to work, but see my remark.
smtLayer/smt.py
Outdated
| log_level_str = "logging.WARN" | ||
| elif level in ("ERROR", "LOGGING.ERROR"): | ||
| log_level_str = "logging.ERROR" | ||
| elif level in ("CRITICAL", "LOGGING.CRITICAL"): |
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.
I know that file log.py contains as well:
elif log_level in ('LOGGING.CRITICAL', 'CRITICAL'):
log_level = logging.CRITICAL
but CRITICAL does not appear anywhere in the code, and is not even documented...
What about removing those 2 lines from log.py instead of adding them here?
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.
@Bischoff yes since "CRITICAL" is not used anywhere in the codebase and is not documented I’ve removed the CRITICAL handling from log.py .
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.
Thank you. I'm a big fan of YAGNI 😛
Signed-off-by: suraksha <p.v.sai.suraksha@ibm.com>
4cede19 to
e4522d8
Compare
Bischoff
left a comment
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.
Thanks for the changes, approving.
|
@P-V-SAI-SURAKSHA I am not a "reviewer with write access", so you might need an approval from @mfriesenegger or @Rajat-0 for the automated tests to be green ;) . |
Task Description:
Solution:
Updated smt.py to read the log level from the configuration file, defaulting to INFO when not set. Logging is now controlled by the configured level, reducing unnecessary log entries and preventing large log files.