-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
plugin: remove Plugin.bind()
, change Plugin.__init__()
and Session.resolve_url()
#4768
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
Conversation
Btw, as a next step (ignore this for now in this PR though), since the session is not a class attribute on the I have stashed some local changes, but these changes also required adding wrappers for direct HTTP-verb-methods of Streamlink's |
be20ca2
to
e2e1eb8
Compare
Rebased to master and fixed merge conflicts introduced by #4771 |
a73cc22
to
e2e1eb8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
and add `Plugin.__init__` signature test for built-in plugins
This changes the way how the Streamlink session and other objects like the plugin cache and logger are stored on each plugin. Previously, those objects were set as class attributes on every `Plugin` class via `Plugin.bind()` when loading plugins via the session's `load_plugins()` method that gets called on initialization. This meant that whenever a new Streamlink session was initialized, references to it (including a dict of every loaded plugin) were set on each `Plugin` class as a class attribute, and Python's garbage collector could not get rid of this memory when deleting the session instance that was created last. Removing `Plugin.bind()`, passing the session via the `Plugin.__init__` constructor, and setting the cache, logger, etc. on `Plugin` instances instead (only one gets initialized by `streamlink_cli`), removes those static references that prevent the garbage collector to work. Since the plugin "module" name now doesn't get set via `Plugin.bind()` anymore, it derives its name via `self.__class__.__module__` on its own, which means a change of the return type of `Streamlink.resolve_url()` is necessary in order to pass the plugin name to `streamlink_cli`, so that it can load config files and initialize plugin arguments, etc. Breaking changes: - Remove `Plugin.bind()` - Pass the `session` instance via the Plugin constructor and set the `module`, `cache` and `logger` on the plugin instance instead. Derive `module` from the actual module name. - Change the return type of `Session.resolve_url()` and include the resolved plugin name in the returned tuple Other changes: - Remove `pluginclass.bind()` call from `Session.load_plugins()` and use the loader's module name directly on the `Session.plugins` dict - Remove initialization check from `Plugin` cookie methods - Update streamlink_cli.main module according to breaking changes - Update tests respectively - Add explicit plugin initialization test - Update tests with plugin constructors and custom plugin names - Move testplugin override module, so that it shares the same module name as the main testplugin module. Rel `Session.load_plugins()` - Refactor most session tests and replace unneeded `resolve_url()` wrappers in favor of calling `session.streams()`
This restores support for custom plugins with constructors which only take the formal `url` argument instead of variable arguments via `*args, **kwargs` that get then passed to the super-class's constructor, so a breaking API change can temporarily be avoided. This is done by wrapping the custom plugin class's MRO with two compatibility classes via `__new__()`. Also add a deprecation message (info log channel) and add tests.
e2e1eb8
to
85f01a6
Compare
Rebased to master now after #4780 was merged. No need to unnecessarily wait for this. This is ready for review now. |
Looks good, thanks for the thorough commit message on e88750, made it a lot more clear. |
and add `Plugin.__init__` signature test for built-in plugins
and add `Plugin.__init__` signature test for built-in plugins
Resolves #4744
Fixes #4358
This introduces breaking changes to the
Plugin.__init__()
method, as well asSession.resolve_url()
andSession.resolve_url_no_redirect()
.See the commit messages of each individual commit for proper descriptions and explanations.
Current
master
:Removal of
Plugin.bind()
:I've decided to add compatibility wrappers for old-style
Plugin
constructors, so that breaking changes for custom plugin implementations can be avoided for a short while and deprecation messages be shown.I still need to update the docs (which I just remembered while writing this). But even then,I want to keep this PR open for a bit, so I can think about all this and check if something's wrong or missing.