session: refactor option getter+setter methods #5076
Merged
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.
This replaces the logic of the
Streamlink.{get,set}_option()
methods with a newStreamlinkOptions
subclass which uses mapped option getters+setters that apply the custom option logic instead.The motivation for this is that this improves the runtime of said methods, as option mappers are looked up in Python dictionaries in constant time, so option names don't have to be checked one after another. The old implementation is also really really bad code-wise and not extensible.
In order to do this, the
Options
class had to be refactored a bit and extended:argparse.Namespace
in mind, so dashes were replaced with underscores. Since Streamlink's session options are defined using dashes, I changed the normalization the other way around, to make it consistent. For various plugin options, this doesn't have any effect (e.g. see various "purge_credentials" options). This is all properly tested and no further changes are required.The third commit then finally cleans up the Streamlink session and moves the special option logic into the newly added
StreamlinkOptions
subclass, and it also adds new deprecation messages (see commit message). The individual getters and setters were not modified except forhttp-disable-dh
, which now has a proper getterhttps-proxy
, which now shows a deprecation message on its getter (still returns the "https" proxy, even though both "http" and "https" get set by the setter, because the proxies can also be defined by requests via env vars)I only added new tests for the mapped options implementation and
http{,s}-proxy
getter. Some of the Streamlink session options are not actually tested, e.g. some HTTP session attributes, but all the getters and setters should be covered, and adding tests for simple setattr/getattr calls isn't too important. Current code coverage (can't permalink with commit-id, so this will change):https://app.codecov.io/gh/streamlink/streamlink/blob/master/src/streamlink/session.py
If this PR gets merged, then this will cause merge conflicts with two of my other open (draft) PRs and those will need a quick update:
Argument.is_global
removal andStreamlink.{get,set}_plugin_option()
removal)