session: replace usage of deprecated imp module #3366

bastimeyer · 2020-11-26T00:34:54Z

Refactor utils.load_module:

move it to the top and add it to __all__
move imports and loader_details to the global scope

Refactor session:

replace plugin imports via imp with utils.load_module
fix incorrect plugin module names
"streamlink.plugin.{name}" -> "streamlink.plugins.{name}"
use log.exception() to print plugin load failures
remove unused print_small_exception

Add / fix tests:

remove session dependency from test_plugins
add proper assertions for plugin modules, methods and their signatures
add tests for invalid and for overridden plugins

Based on the changes of #2570

Things to note

I've decided to keep the implementation of importlib.machinery.FileFinder in the utils module and use that for loading the plugins, as I had some issues with the module names while trying out the pkgutils-based imports of session: use importlib to load plugins in Python 3 #2570, which led to plugins log calls not being recognized by the StreamlinkLogger, as their module names did not begin with streamlink..
test_session is really messy as it imports everything in tests.plugins, including all plugin test modules, which are irrelevant there. The actual testplugins should be moved somewhere else. Mocking certain things is also impossible when everything is run in the test's setUp, and I couldn't mock the log call of the plugin override without patching the whole test class which would require adding an unused argument to every test method. That's why plugin load failures are also not tested.
Why does plugin.api.support_plugin even exist? It's not being used and it looks totally unnecessary. This module is the reason why utils.load_module existed, but now it's being used by session as well.

Refactor `utils.load_module`: - move it to the top and add it to `__all__` - move imports and loader_details to the global scope Refactor `session`: - replace plugin imports via `imp` with `utils.load_module` - fix incorrect plugin module names "streamlink.plugin.{name}" -> "streamlink.plugins.{name}" - use `log.exception()` to print plugin load failures - remove unused `print_small_exception` Add / fix tests: - remove session dependency from test_plugins - add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins

back-to · 2020-11-27T15:17:49Z

streamlink/src/streamlink/session.py

Lines 413 to 417 in 5e7a04b

    
           try: 
        
               mod = load_module(module_name, path) 
        
           except ImportError: 
        
               log.exception(f"Failed to load plugin {name} from {path}\n") 
        
               continue

it is not always ImportError which will change the behavior of Streamlink a bit,
broken plugins will error Streamlink instead of skipping the plugin.

Not that important but just to be noted.

Why does plugin.api.support_plugin even exist?

we don't need it, could probably be removed,
same for the common_ plugins

bastimeyer · 2020-11-27T17:19:58Z

it is not always ImportError

According to the docs, both the finder and loader will raise an ImportError (or one of their subclasses) on failure. Same does the load_module method if there's no spec or spec.loader. What else is there to be aware of?

back-to · 2020-11-27T18:29:31Z

Example: if you sideload a broken plugin


Traceback (most recent call last):
  File "venv/bin/streamlink", line 33, in <module>
    sys.exit(load_entry_point('streamlink', 'console_scripts', 'streamlink')())
  File "src/streamlink_cli/main.py", line 1012, in main
    setup_plugins(args.plugin_dirs)
  File "src/streamlink_cli/main.py", line 739, in setup_plugins
    load_plugins([PLUGINS_DIR])
  File "src/streamlink_cli/main.py", line 640, in load_plugins
    streamlink.load_plugins(directory)
  File "src/streamlink/session.py", line 414, in load_plugins
    mod = load_module(module_name, path)
  File "src/streamlink/utils/__init__.py", line 25, in load_module
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 779, in exec_module
  File "<frozen importlib._bootstrap_external>", line 916, in get_code
  File "<frozen importlib._bootstrap_external>", line 846, in source_to_code
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/.../.config/streamlink/plugins/testplugin.py", line 36
    __plu gin__ = Teamliquid
          ^
SyntaxError: invalid syntax

Traceback (most recent call last):
  File "venv/bin/streamlink", line 33, in <module>
    sys.exit(load_entry_point('streamlink', 'console_scripts', 'streamlink')())
  File "src/streamlink_cli/main.py", line 1012, in main
    setup_plugins(args.plugin_dirs)
  File "src/streamlink_cli/main.py", line 739, in setup_plugins
    load_plugins([PLUGINS_DIR])
  File "src/streamlink_cli/main.py", line 640, in load_plugins
    streamlink.load_plugins(directory)
  File "src/streamlink/session.py", line 414, in load_plugins
    mod = load_module(module_name, path)
  File "src/streamlink/utils/__init__.py", line 25, in load_module
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 779, in exec_module
  File "<frozen importlib._bootstrap_external>", line 916, in get_code
  File "<frozen importlib._bootstrap_external>", line 846, in source_to_code
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/.../.config/streamlink/plugins/testplugin.py", line 19
    def _get_streams(self):
                          ^
IndentationError: unindent does not match any outer indentation level

bastimeyer · 2020-11-27T19:32:13Z

If you want, I can "revert" this change of logic by adding a secondary except SyntaxError: block and adding back print_small_exception, so that custom plugin errors can be written to stderr while continuing Streamlink's execution.

I personally am OK though with Streamlink failing if code of a custom plugin causes a parsing error.

back-to · 2020-11-27T19:37:42Z

If you want, I can "revert" this change

no it's fine, can stay like this.

back-to · 2020-12-03T17:15:12Z

well, lets test this with a large user base.

bastimeyer · 2020-12-03T17:24:14Z

Maybe we should add a note to the sideloading plugins section in the docs that plugins which fail parsing will make Streamlink terminate its process beginning with 2.0.0.
I'll open a PR later or tomorrow. Maybe I'll also refactor the tests (see my comment above).

Will also take a look at the support_plugin thing again. I don't think it makes sense keeping it.

since streamlink#3366 https://github.com/streamlink/streamlink/blob/5e7a04b006d0e4d7a884b13bf2aa8047791b837b/src/streamlink/session.py#L412 the logger is `[plugins.afreeca]` which was `[plugin.afreeca]` before update the cls.logger so it will be `[plugins.afreeca]` also after this PR ``` [plugins.afreeca][debug] Restored cookies: ... ... [plugins.afreeca][debug] Attempting to authenticate using cached cookies ```

since #3366 https://github.com/streamlink/streamlink/blob/5e7a04b006d0e4d7a884b13bf2aa8047791b837b/src/streamlink/session.py#L412 the logger is `[plugins.afreeca]` which was `[plugin.afreeca]` before update the cls.logger so it will be `[plugins.afreeca]` also after this PR ``` [plugins.afreeca][debug] Restored cookies: ... ... [plugins.afreeca][debug] Attempting to authenticate using cached cookies ```

since streamlink#3366 https://github.com/streamlink/streamlink/blob/5e7a04b006d0e4d7a884b13bf2aa8047791b837b/src/streamlink/session.py#L412 the logger is `[plugins.afreeca]` which was `[plugin.afreeca]` before update the cls.logger so it will be `[plugins.afreeca]` also after this PR ``` [plugins.afreeca][debug] Restored cookies: ... ... [plugins.afreeca][debug] Attempting to authenticate using cached cookies ```

- add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins based on streamlink#3366

- add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins based on streamlink#3366 Update test_plugins.py

bastimeyer added the cleanup label Nov 26, 2020

bastimeyer requested a review from beardypig November 26, 2020 00:35

bastimeyer force-pushed the deprecations/imp branch from 5ccfe38 to 5e7a04b Compare November 26, 2020 02:39

bastimeyer mentioned this pull request Nov 26, 2020

The road towards 2.0.0 #3235

Closed

25 tasks

OlMon mentioned this pull request Dec 3, 2020

Python 3.9 problems #3387

Closed

2 tasks

back-to merged commit 156d870 into streamlink:master Dec 3, 2020

This was referenced Dec 4, 2020

docs: add warning to plugin sideloading section #3391

Merged

plugin.api: remove support_plugin #3398

Merged

back-to mentioned this pull request Dec 14, 2020

plugins.plugin: use the same cls.logger 'plugins' #3417

Merged

bastimeyer deleted the deprecations/imp branch January 19, 2021 23:16

Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Feb 8, 2021

tests: Add / fix tests

132d73f

- add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins based on streamlink#3366

Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Feb 8, 2021

tests: Add / fix tests

cd5389a

- add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins based on streamlink#3366

Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Feb 8, 2021

tests: Add / fix tests

9717c58

- add proper assertions for plugin modules, methods and their signatures - add tests for invalid and for overridden plugins based on streamlink#3366

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

session: replace usage of deprecated imp module #3366

session: replace usage of deprecated imp module #3366

Uh oh!

bastimeyer commented Nov 26, 2020

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

bastimeyer commented Nov 27, 2020

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

bastimeyer commented Nov 27, 2020 •

edited

Loading

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

back-to commented Dec 3, 2020

Uh oh!

bastimeyer commented Dec 3, 2020

Uh oh!

Uh oh!

Uh oh!

session: replace usage of deprecated imp module #3366

session: replace usage of deprecated imp module #3366

Uh oh!

Conversation

bastimeyer commented Nov 26, 2020

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

bastimeyer commented Nov 27, 2020

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

bastimeyer commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

back-to commented Nov 27, 2020

Uh oh!

back-to commented Dec 3, 2020

Uh oh!

bastimeyer commented Dec 3, 2020

Uh oh!

Uh oh!

bastimeyer commented Nov 27, 2020 •

edited

Loading