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.

Sign up for GitHub

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

Jump to bottom

Show DeprecationWarnings as INFO messages #2907

Merged
merged 1 commit into from
Aug 1, 2022
Merged

Show DeprecationWarnings as INFO messages #2907

merged 1 commit into from
Aug 1, 2022

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Jul 31, 2022

(previously they were not shown at all)

This works for both extensions and plugins -- and for MkDocs itself.
It will enable plugin developers to see warnings in advance. Or users to report such warnings to plugin developers.

Ref #2892, Python-Markdown/markdown#1277

Example session:

$ .venv/bin/pip install 'mdx_gh_links==0.2' 'Markdown==3.3.7'
$ PYTHONPATH=. .venv/bin/mkdocs serve                        
INFO     -  Building documentation...
INFO     -  DeprecationWarning: 'etree' is deprecated. Use 'xml.etree.ElementTree' instead.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/mdx_gh_links.py", line 35, in <module>
                from markdown.util import etree
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/util.py", line 475, in __getattr__
                warnings.warn(
INFO     -  DeprecationWarning: Using setitem to register a processor or pattern is deprecated. Use the `register` method instead.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/mdx_gh_links.py", line 126, in extendMarkdown
                md.inlinePatterns['issue'] = IssuePattern(self.getConfigs(), md)
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/util.py", line 393, in __setitem__
                warnings.warn(
INFO     -  DeprecationWarning: The 'md_globals' parameter of 'mdx_gh_links.GithubLinks.extendMarkdown' is deprecated.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/core.py", line 125, in registerExtensions
                ext._extendMarkdown(self)
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/extensions/__init__.py", line 82, in _extendMarkdown
                warnings.warn(
INFO     -  Cleaning site directory
INFO     -  Documentation built in 0.51 seconds

cc @ultrabug @pawamoy

@oprypin
Show DeprecationWarnings as INFO messages
67a1dbf 
(previously they were not shown at all)

This works for both extensions and plugins -- and for MkDocs itself.
It will enable plugin developers to see warnings in advance. Or users to report such warnings to plugin developers.
@oprypin oprypin mentioned this pull request Jul 31, 2022
Closed
ultrabug
ultrabug approved these changes Aug 1, 2022
View reviewed changes
Copy link
Member

@ultrabug ultrabug left a comment

Choose a reason for hiding this comment

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

Good job, I find that very elegant

@pawamoy
Copy link
Contributor

pawamoy commented Aug 1, 2022 •
edited
Loading

Just to understand: the change basically makes sure warnings are shown to the user by logging them as INFO? If users already have warnings shown, will they see them twice (once as a regular warning and a second time as an info log)? Also, good call on making them INFO level and not WARNING 👍

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022 •
edited
Loading

There are 2 mechanisms at play here, both of them configured totally separately and both of them affected by this PR:

  1. Whether a warning is enabled.
    Regardless of how the environment is configured, this forcibly enables all DeprecationWarnings.
    If a warning is enabled, it proceeds to the "show warning" part.

  2. How to show a warning.
    This forcibly replaces the way of displaying all warnings - to go to the log mechanism instead of the default output.
    There is no way for the default mechanism to kick in anymore.
    This is the recommended approach https://docs.python.org/3/library/warnings.html#warnings.showwarning

@pawamoy
Copy link
Contributor

pawamoy commented Aug 1, 2022

Thanks, that answers my questions :) perfect.

@oprypin oprypin merged commit 3ffe21d into mkdocs:master Aug 1, 2022
@pawamoy
Copy link
Contributor

pawamoy commented Aug 1, 2022 •
edited
Loading

Actually I'm not sure if hiding regular warnings in favor of INFO logs is a good idea. I mean, maybe keeping the duplication would be better. In the common case, that's a few more lines of INFO logs few will read, in the worst case, users that were relying on regular warnings don't get their pytest run fail anymore, and I believe that's less chance these users will report these deprecations to the upstream packages. I hope I'm wrong though. The INFO logs are pretty visible so I guess it's fine.

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022

users that were relying on regular warnings don't get their pytest run fail anymore,

Why would that happen? This affects only full runs of mkdocs

I mean, maybe keeping the duplication would be better.

How is printing the exact same message twice better? There is no possible case where only one of them would kick in - it's either 2 or 0.

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022

There is no possible case

Well, mkdocs -q hides these messages, but that's a desired thing.

Other than that, there's no difference, both are just text in stderr

@pawamoy
Copy link
Contributor

pawamoy commented Aug 1, 2022

Hmmm yeah I don't think a lot of users/plugin developers run MkDocs commands through pytest with warnings enabled anyway 🤔
Always happy to be proven wrong.
I just know warnings are a nice way to, well, warn users, and worry about the effects of replacing them with logs, which don't react to the same mechanisms. But I'm probably overthinking here. I trust your decisions @oprypin. And I'm slowly getting back to open source so don't mind me too much.

@llwt llwt mentioned this pull request Jan 25, 2023
Closed
@wilhelmer wilhelmer mentioned this pull request Mar 10, 2023
Closed
@kcgthb kcgthb mentioned this pull request May 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ultrabug ultrabug ultrabug approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oprypin @pawamoy @ultrabug