Skip to content

Conversation

@bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Jan 31, 2024

Ref #4741
Part 2/x

This fixes some long overdue issues with the session and http_session modules, and it's also a necessary preparation for the lazy plugins loader implementation.


Motivation

The plugin.api.http_session module is not actually part of the "plugin API". It is one of the main dependencies of the session module, which means it doesn't belong into the plugin.api package. The HTTPSession is also (obviously) used in many other places, like in the stream implementations for example, and it's always referenced using the Streamlink session object.

The http_session module was added ~10 years ago during the Livestreamer era via 936e66d, so plugins could share a requests.Session object between HTTP requests, which were previously done directly via requests (with an optional session passed to each request).

Module re-organization

The following commits re-organize the module structure (see the commit message for more details):

  1. Turn streamlink.session into a sub-package, with the Streamlink class being defined in the streamlink.session.session module, but re-exported in streamlink.session
  2. Clean up an unused class from the 2.7 deprecation era
  3. Move the StreamlinkOptions subclass of Options to the streamlink.session.options module, so it doesn't bloat up the session module. This also moves all the option definitions and docs into that module, which is much better.
  4. Move the HTTPSession subclass of requests.Session to the streamlink.session.http module.
  5. Add back the streamlink.plugin.api.http_session module for compatibility reasons.

Before:
https://github.com/streamlink/streamlink/tree/6.5.1/src/streamlink

- plugin
  - api: HTTPSession
    - http_session: HTTPSession, TLSNoDHAdapter, TLSSecLevel1Adapter
- session: Streamlink, StreamlinkOptions

After:
https://github.com/bastimeyer/streamlink/tree/836e30029d36ee27ad13ba511127c24c511cce44/src/streamlink

- plugin
  - api: HTTPSession (deprecated compat re-export - imported on access time)
    - http_session: HTTPSession, TLSNoDHAdapter, TLSSecLevel1Adapter (deprecated compat re-exports)
- session: Streamlink
  - session: Streamlink
  - options: StreamlinkOptions
  - http: HTTPSession, TLSNoDHAdapter, TLSSecLevel1Adapter

Future benefits

This allows extending the streamlink.session sub-package with more modules, e.g. for the lazy plugins loader, which will move plugins loading and resolving logic from the main Streamlink session class.

Notes

streamlink.plugin.api.useragents unfortunately is still a dependency of streamlink.session.http. I'm not sure if changing this is worth it, because if we'd move that module, then a compatibility module with re-exports would need to be added anyway. This could maybe be done in the future. It's not too important though.

The HTTPSession class was never meant to be imported by any plugin, so those references have been removed from streamlink.plugin.api (and not been added back, unlike the HTTPAdapter classes). Other than that, everything should be compatible. I don't consider this a breaking change. Everything's compatible, with deprecation warnings when importing plugin.api.HTTPSession or plugin.api.http_session.{HTTPSession,TLSNoDHAdapter,TLSSecLevel1Adapter}.

The TLSNoDHAdapter and TLSSecLevel1Adapter classes are currently re-exported in streamlink.session, because streamlink.session.http is considered a private module now. I'm not super happy with this, because it's HTTPSession related. I don't really mind plugins importing from the http module though. There's only one plugin which imports one of these custom adapters, and that's FilmOn. I'll have to think about it.

@bastimeyer
Copy link
Member Author

@gravyboat
Copy link
Member

Code changes look good.

There's only one plugin which imports one of these custom adapters, and that's FilmOn. I'll have to think about it.

Let's not compromise for a single plugin if it doesn't make sense to do so.

- Create `streamlink.session` sub-package and move `session` module
  - Update/fix logger name
- Create `tests.session` sub-package and move `test_session` module
  - Fix monkeypatch paths
- Run session tests with a higher priority
- Move `StreamlinkOptions` from `streamlink.session`
  to `streamlink.session.options`
- Move docstring containing options list from `Streamlink.set_option()`
  to `StreamlinkOptions` and add this class to the docs
- Move default options from `Streamlink` constructor
  to `StreamlinkOptions._DEFAULTS`, so all options data is defined
  in the `streamlink.session.options` module
- Move session options tests to `tests.session.test_options`
  and change order of tests
@bastimeyer bastimeyer force-pushed the session/move-to-package branch from 836e300 to 6788d8d Compare February 2, 2024 23:48
@bastimeyer bastimeyer changed the title session: turn session module into a sub-package session: re-organize session and http_session modules Feb 2, 2024
@bastimeyer bastimeyer force-pushed the session/move-to-package branch from 6788d8d to 81a3fd1 Compare February 2, 2024 23:50
- Move `streamlink.plugin.api.http_session` module
  to `streamlink.session.http`
- Deprecate old (and unnecessary) `HTTPSession` export
  in `streamlink.plugin.api`
- Fix custom `HTTPAdapter` imports in plugins
- Fix imports and fix monkeypatch paths in tests

In the next commit:
Restore the `streamlink.plugin.api.http_session` module and re-export
`HTTPSession` and the `HTTPAdapter` classes with deprecation warnings.
@bastimeyer bastimeyer force-pushed the session/move-to-package branch from 81a3fd1 to fb08aad Compare February 3, 2024 00:14
@bastimeyer bastimeyer merged commit ecf182a into streamlink:master Feb 3, 2024
@bastimeyer bastimeyer deleted the session/move-to-package branch February 3, 2024 00:20
@W4RdZ W4RdZ mentioned this pull request May 9, 2024
4 tasks
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.

2 participants