Skip to content

Application: add logging_config trait#698

Merged
blink1073 merged 3 commits intoipython:mainfrom
oliver-sanders:688
Apr 11, 2022
Merged

Application: add logging_config trait#698
blink1073 merged 3 commits intoipython:mainfrom
oliver-sanders:688

Conversation

@oliver-sanders
Copy link
Contributor

@oliver-sanders oliver-sanders commented Apr 1, 2022

Closes #688

Allows fine configuration of logging via a logging.config.dictConfig, useful for configuring additional log handlers/formatters and advanced logging setups.

  • Fixes doctests and gets them running with pytest.
  • Unifies the pre-existing logging configurations into a standard "base" logging configuration dict.
  • Adds a new trait for providing a logging config dict which is merged into the "base" configuration allowing adding configs as well as redefining existing ones.
  • Changes the default logger level from WARN to DEBUG and the default handler level from undefined to WARN (I think the log_level should apply to the handler not the logger otherwise).

@blink1073
Copy link
Contributor

blink1073 commented Apr 8, 2022

Hi @oliver-sanders, it looks like the switch to using black caused some merge conflicts.

The recommended way to handle this is to:

Run Black on modified files, keep a copy aside, start a new branch from "main", move the modified files back into place, and commit the result.

@oliver-sanders
Copy link
Contributor Author

oliver-sanders commented Apr 11, 2022

Ok, that should do it, have kept a copy of the original branch for reference - https://github.com/ipython/traitlets/compare/main...oliver-sanders:688.20220411T1120?expand=1

blink1073
blink1073 approved these changes Apr 11, 2022
View reviewed changes
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

@blink1073
Copy link
Contributor

blink1073 commented Apr 11, 2022

Looks like another doctest skip is needed:

 =================================== FAILURES ===================================
_______________ [doctest] traitlets.tests._warnings.all_warnings _______________
023     ...     warnings.warn(RuntimeWarning("bar"))
024 
025     We raise the warning once, while the warning filter is set to "once".
026     Hereafter, the warning is invisible, even with custom filters:
027     >>> with warnings.catch_warnings():
028     ...     warnings.simplefilter('once')
029     ...     foo()
030 
031     We can now run ``foo()`` without a warning being raised:
032     >>> from numpy.testing import assert_warns
UNEXPECTED EXCEPTION: ModuleNotFoundError("No module named 'numpy'")
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.12/x64/lib/python3.7/doctest.py", line 1337, in __run
    compileflags, 1), test.globs)
  File "<doctest traitlets.tests._warnings.all_warnings[3]>", line 1, in <module>
ModuleNotFoundError: No module named 'numpy'

@oliver-sanders
fix doctests
47c5837
@oliver-sanders
logging: add a logging_config trait
b5b5085 
* Closes ipython#688
* Allows fine configuration of logging via a `logging.config.dictConfig`.
* Changes the default log level from WARN to DEBUG and the default log
  handler level from undefined to WARN.
@oliver-sanders
Copy link
Contributor Author

oliver-sanders commented Apr 11, 2022

Looks like another doctest skip is needed:

Test was failing in CI because numpy dependency would be needed, added skips for now.

@blink1073
Copy link
Contributor

blink1073 commented Apr 11, 2022

Thanks again! The downstream failure is being handled in #701

@blink1073 blink1073 merged commit dcdec78 into ipython:main Apr 11, 2022
@oliver-sanders oliver-sanders mentioned this pull request Apr 11, 2022
Open
@oliver-sanders oliver-sanders mentioned this pull request Apr 20, 2022
Merged
7 tasks
@Zsailer Zsailer mentioned this pull request May 12, 2022
Closed
@Zsailer
Copy link
Member

Zsailer commented May 12, 2022

I think this might have broken some things: #722

I'll try to dive into this today/tomorrow.

@Zsailer Zsailer mentioned this pull request May 12, 2022
Closed
@oliver-sanders oliver-sanders mentioned this pull request May 13, 2022
Merged
@consideRatio consideRatio mentioned this pull request Jun 10, 2022
Closed
@yuvipanda yuvipanda mentioned this pull request Nov 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@blink1073 blink1073 blink1073 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

log: allow log handlers to be configured

3 participants

@oliver-sanders @blink1073 @Zsailer