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.

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

Improve the logging tests and add support for resetting the logger #1716

Merged

Conversation

mdemoret-nv
Copy link
Contributor

Description

  • Allows the logger to be "reset" which is necessary to avoid duplicate logging handlers if configure_logging is called multiple times
  • Updates all of the logging tests to parameterize the log level and better check if the handlers are actually being called.

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@mdemoret-nv mdemoret-nv added non-breaking Non-breaking change improvement Improvement to existing functionality labels May 21, 2024
@mdemoret-nv mdemoret-nv requested a review from a team as a code owner May 21, 2024 22:51
Copy link
Contributor

@dagardner-nv dagardner-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a few changes the clear_handlers and pytest_runtest_teardown methods in tests/conftest.py could be updated to make use of the new reset_logging method.

morpheus/utils/logger.py Outdated Show resolved Hide resolved
morpheus/utils/logger.py Outdated Show resolved Hide resolved
tests/test_logger.py Outdated Show resolved Hide resolved
@dagardner-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 21c1694 into nv-morpheus:branch-24.06 Jun 4, 2024
12 checks passed
dagardner-nv pushed a commit to dagardner-nv/Morpheus that referenced this pull request Jun 4, 2024
…v-morpheus#1716)

- Allows the logger to be "reset" which is necessary to avoid duplicate logging handlers if `configure_logging` is called multiple times
- Updates all of the logging tests to parameterize the log level and better check if the handlers are actually being called.

- I am familiar with the [Contributing Guidelines](https://github.com/nv-morpheus/Morpheus/blob/main/docs/source/developer_guide/contributing.md).
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Michael Demoret (https://github.com/mdemoret-nv)
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)

URL: nv-morpheus#1716
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement to existing functionality non-breaking Non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants