[sonic-yang-mgmt] Adding flag to disable/enable log printing#9659
Merged
ghooo merged 7 commits intosonic-net:masterfrom Jan 5, 2022
Merged
[sonic-yang-mgmt] Adding flag to disable/enable log printing#9659ghooo merged 7 commits intosonic-net:masterfrom
ghooo merged 7 commits intosonic-net:masterfrom
Conversation
ghooo
changed the title
ghooo
force-pushed
the
dev/mghoneim/sy_loging
branch
from
8f520ae to
42ad27c
Compare
ghooo
changed the title
ghooo
changed the title
logging_disabled flag to sonic_yang ctor
ghooo
force-pushed
the
dev/mghoneim/sy_loging
branch
from
33043df to
42ad27c
Compare
ghooo
changed the title
logging_disabled flag to sonic_yang ctorenable_logging disable_logging methods to sonic_yang
qiluo-msft
reviewed
Dec 30, 2021
src/sonic-yang-mgmt/sonic_yang.py
Outdated
| self.syslogging_disabled = False | ||
|
|
||
| def sysLog(self, debug=syslog.LOG_INFO, msg=None, doPrint=False): | ||
| if self.syslogging_disabled: |
Collaborator
Contributor
Author
There was a problem hiding this comment.
my intention is to disable logging
Contributor
Author
There was a problem hiding this comment.
I think it will be hard to read the logs as well if we keep printing errors during a change validation
Contributor
Author
There was a problem hiding this comment.
I see why it might be confusing, the name of the property syslogging_disabled but it is disabling it for both syslog and print. I got the name of the property from the name of the method sysLog.
Contributor
Author
There was a problem hiding this comment.
renamed to print_log_enabled, and it only controls log printing
ghooo
changed the title
enable_logging disable_logging methods to sonic_yang
ghooo
changed the title
ghooo
changed the title
praveen-li
approved these changes
Jan 4, 2022
Member
praveen-li
left a comment
praveen-li
left a comment
There was a problem hiding this comment.
LGTM, BTW I cannot recall why the name of argument is 'debug' in sysLog function. We can change it to 'level' or 'logLevel', if needed.
def sysLog(self, debug=syslog.LOG_INFO, msg=None, doPrint=False):
Approved, even if above is not addressed. Thx
qiluo-msft
approved these changes
Jan 4, 2022
Contributor
Author
|
@praveen-li let me rename the flag in a separate PR, the build takes a very long time to finish and I have an ask I need to finish soon. |
qiluo-msft
added
Request for 202111 Branch
judyjoseph
pushed a commit
that referenced
this pull request
Jan 9, 2022
#### Why I did it Fixes sonic-net/sonic-utilities#1871 From [generic-config-updater](https://github.com/Azure/sonic-utilities/tree/master/generic_config_updater) we call `sonic-yang-mgmt` multiple times in order to check a certain change to ConfigDb is valid or not. It is expected for some changes to be invalid, so always printing errors from `sonic-yang-mgmt` makes the output hard to read. In this PR, we are adding a way to control if logs should be printed or not. #### How I did it - Added `print_log_enabled` flag to sonic_yang ctor - Converted all `print` statements to `sysLog(..., doPrint=True)` #### How to verify it unit-test passing means the change did not break logs. #### Info about libyang logging libyang provides an extensive logging logic which can support a lot of scenarios: - ly_log_level: setting logging level - LY_LLERR - LY_LLWRN - ... - ly_set_log_clb: setting log callback to customize the default behavior which is printing the msgs - ly_log_options: setting logging options - LY_LOLOG: If callback is set use it, otherwise just print. If flag is not set, do nothing. - ... For more info refer to: - https://netopeer.liberouter.org/doc/libyang/devel/html/group__logopts.html#gaff80501597ed76344a679be2b90a1d0a - https://netopeer.liberouter.org/doc/libyang/devel/html/group__log.html#gac88b78694dfe9efe0450a69603f7eceb #### What's next? Consume the new flag `print_log_enabled` in [generic-config-updater](https://github.com/Azure/sonic-utilities/tree/master/generic_config_updater) to reduce the logging clutter. #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 #### Description for the changelog <!-- Write a short (one line) summary that describes the changes in this pull request for inclusion in the changelog: --> #### A picture of a cute animal (not mandatory but encouraged)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
Fixes sonic-net/sonic-utilities#1871
From generic-config-updater we call
sonic-yang-mgmtmultiple times in order to check a certain change to ConfigDb is valid or not. It is expected for some changes to be invalid, so always printing errors fromsonic-yang-mgmtmakes the output hard to read.In this PR, we are adding a way to control if logs should be printed or not.
How I did it
print_log_enabledflag to sonic_yang ctorprintstatements tosysLog(..., doPrint=True)How to verify it
unit-test passing means the change did not break logs.
Info about libyang logging
libyang provides an extensive logging logic which can support a lot of scenarios:
For more info refer to:
What's next?
Consume the new flag
print_log_enabledin generic-config-updater to reduce the logging clutter.Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)