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

Use recommended dictConfig for logging configuration #850

Merged

Conversation

glatterf42
Copy link
Collaborator

This PR is in tandem with iiasa/ixmp4#83, where you can find an extensive description of the underlying issue. For pyam, there is no server.conf, so these parts can be disregarded.

Does this PR need to be mentioned in the release notes?
Also, can we please have a quick patch release after merging this?

@glatterf42 glatterf42 self-assigned this Apr 19, 2024
@glatterf42 glatterf42 added the bug label Apr 19, 2024
Copy link

codecov bot commented Apr 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.0%. Comparing base (ddbb88e) to head (a058755).
Report is 14 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #850     +/-   ##
=======================================
- Coverage   95.0%   95.0%   -0.1%     
=======================================
  Files         64      63      -1     
  Lines       6134    6151     +17     
=======================================
+ Hits        5828    5844     +16     
- Misses       306     307      +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glatterf42
Copy link
Collaborator Author

I can't re-run the failing test, but given that the other tests (especially for py3.12) passed, I'd assume that this error is flaky and not related to the changes made in this PR.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thanks, a few minor suggestions inline...

pyam/logging.py Outdated Show resolved Hide resolved
pyam/logging.json Outdated Show resolved Hide resolved
pyam/logging.py Outdated Show resolved Hide resolved
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thanks @glatterf42, hope that this will resolve some downstream issues...

@danielhuppmann danielhuppmann merged commit 1a9c05d into IAMconsortium:main Apr 19, 2024
14 checks passed
@glatterf42 glatterf42 deleted the fix/deleting-root-logger-handlers branch April 19, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants