Skip to content

session.resolve_url: return plugin class + URL #4163

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

Merged

Conversation

bastimeyer
Copy link
Member

Breaking change:
Instead of resolving a plugin instance in Streamlink.resolve_url(url)
from the provided input URL (which can be redirected), resolve the
plugin class and the resulting URL, and cache the tuple. Also affects
Streamlink.resolve_url_no_redirect(url).

The main reason for this change is streamlink_cli and how the plugin
options get set. The plugin options need to be set on the class before
it gets initialized, so that the instance can access the options in its
constructor method. This also fixes any kind of state stored on the
plugin instance.

  • resolve and cache a tuple of the plugin class and resulting URL
  • initialize plugin in streamlink_cli after applying its options
  • remove plugin variable from global scope in streamlink_cli
  • fix, rewrite and add tests

Resolves #4091
Resolves #4161


Notes

Updating the URL in streamlink_cli's argument parser setup and removing the redirect logic from Streamlink.resolve_url() (as suggested here: #4091 (comment)) didn't seem to be trivial because of how the session is set up and how plugin config files are handled in setup_config_args(), which gets called twice before handle_url() gets called. The current solution should be good enough, and the logic of streamlink.streams() does not get affected because of that, which is good.

  1. I didn't add a special test for checking if plugin options are available in the plugin constructor when initialized via streamlink_cli, but you can check for yourself that it's working by adding a breakpoint in one of the plugin constructors.
  2. Since the potentially redirected URL gets returned now, another log message could be added in handle_url() if the input and resolved URL are different.
  3. The session tests now can't make accidental HTTP requests due to mocking all requests
  4. I've added a second commit for adding more type annotations (anything related to handle_url())

Breaking change:
Instead of resolving a plugin instance in `Streamlink.resolve_url(url)`
from the provided input URL (which can be redirected), resolve the
plugin class and the resulting URL, and cache the tuple. Also affects
`Streamlink.resolve_url_no_redirect(url)`.

The main reason for this change is streamlink_cli and how the plugin
options get set. The plugin options need to be set on the class before
it gets initialized, so that the instance can access the options in its
constructor method. This also fixes any kind of state stored on the
plugin instance.

- resolve and cache a tuple of the plugin class and resulting URL
- initialize plugin in streamlink_cli after applying its options
- remove plugin variable from global scope in streamlink_cli
- fix, rewrite and add tests
Copy link
Member

@beardypig beardypig left a comment

Choose a reason for hiding this comment

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

I haven't had chance to look at this in depth, but the diff looks good. I had done it in a similar way on another branch, but I didn't have time to update the tests. I had also started on a test for the plugin options being available in __init__.

@back-to back-to merged commit 0ed1d67 into streamlink:master Nov 12, 2021
@bastimeyer bastimeyer deleted the session/resolve-url-plugin-class branch November 12, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin keep init if running too many sessions in the same time Plugin options do not get set during plugin initialization
3 participants