logger: capture warnings #5072
Merged
logger: capture warnings #5072
Conversation
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
bastimeyer
force-pushed
the
logger/warnings
branch
from
8386af3 to
52cf7f2
Compare
bastimeyer
force-pushed
the
logger/warnings
branch
3 times, most recently
from
0d347e1 to
75ddadc
Compare
|
Decided to change the warning message format, because of warnings from dependencies which should contain all the info necessary to find the cause of those warnings. The paths of Streamlink deprecation warnings could be suppressed in a future change. Warnings should be subclassed anyway, as mentioned, so this could be done in this step. New format: Examples: Streamlink deprecation warning InsecureRequestWarning (urllib3) Current master: Streamlink deprecation warning InsecureRequestWarning (urllib3) |
Add support for capturing warnings and logging them via Streamlink's root logger on the warning log level. Use custom `WarningLogRecord`s with a custom warning message format, and replace the record's logger name with "warnings" and use the name of the warning type as log level name when showing warnings. This enables having proper `DeprecationWarning`s and `FutureWarning`s in the code instead of just using the logger and its warning log level, and those warnings can be filtered via the regular filtering mechanisms. For example: ``` [warnings][deprecationwarning] Calling this method is deprecated [warnings][futurewarning] Using this config file path is deprecated ```
- Replace all deprecation warning log messages with `FutureWarning`s - Fix deprecated session options in tests - Update and fix tests
bastimeyer
force-pushed
the
logger/warnings
branch
from
75ddadc to
8a46646
Compare
|
Rebased to master and added This should cover everything now. |
- Add `StreamlinkWarning` and `StreamlinkDeprecationWarning` and replace `FutureWarning`s - Don't include the warning's origin in the warning logger if it's a subclass of `StreamlinkWarning` - Update tests
bastimeyer
force-pushed
the
logger/warnings
branch
from
8a46646 to
b64de6f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See #4785
This makes Streamlink's root logger capture warnings (disabled by default, enabled via streamlink_cli) and it replaces various warning log messages for deprecated (user-facing) stuff with
FutureWarnings.https://docs.python.org/3/library/exceptions.html#warnings
https://docs.python.org/3/library/warnings.html#warning-categories
The reason for this is that this allows for finer control over the kind of deprecation messages (API stuff for devs, or other stuff for end-users) and it uses the standard interfaces for warnings, which is preferred. Warnings from dependencies can also be caught this way, but I'm not sure how useful this is, because I modified the default warning message format, so it doesn't include the file+line of the warning, because it's not useful for Streamlink's deprecation messages (
FutureWarning). This can be changed later on if it causes problems. I only realized this while writing this PR text.Warnings can always be filtered via Python's
-Wparameter (in addition to the regular log level of Streamlink's root logger):https://docs.python.org/3/library/warnings.html#describing-warning-filters
The changes of the logger's warning messages are this, when printed from a caught warning, not from regular
warning()calls:So instead of the logger name and the "warning" log level, it logs "warnings" as logger name and uses the name of the warning as log level name.
As a next step, dedicated warning subclasses could be added for reusable code and better deprecation messages with links to the docs, and with better warning names, etc.
Opening as a draft for now...