Skip to content
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

plugin.api.http_session: remove parse_* methods #4803

Conversation

bastimeyer
Copy link
Member

The parse_{cookies,headers,query_params} methods were added when the
subclass of requests.Session was implemented in order to support
setting cookies, headers and query parameters via k1=v1;k2=v2 strings
(in addition to key-value dicts) via the session API and via the CLI:

Since these methods implement logic purely for the Streamlink session
interface and are not meant to be called by any plugin or stream
implementations which use the session's HTTPSession instance, they
should be removed. Cookies, headers and query string parameters should
be set directly on their respective HTTPSession attributes:

  • cookies: instance of requests.cookies.RequestsCookieJar
  • headers: instance of requests.structures.CaseInsensitiveDict
  • params: instance of dict

Also, at least in regards to HTTP headers, the key=value syntax
does not reflect the syntax of raw HTTP requests/responses or interfaces
of other tools like cURL, etc., so having these methods on the
HTTPSession class makes it unnecessarily confusing. The method names
themselves are also confusing, as they suggest that the input gets
parsed and that some result gets returned, which is wrong.

This commit therefore moves the k1=v1;k2=v2 string logic from the
http_session module to the session module where it belongs and it
also simplifies the option setter.


This might be a breaking change, not 100% sure, but since this isn't really advertised as a public interface, I don't see the need to document this removal, even though it's been implemented on the HTTPSession for almost 9 years.

This will only affect 3rd party code which is calling these HTTPSession methods. The key=value syntax is still kept and remains the same on the CLI and Streamlink session API. See the added tests.

Merging this will introduce a conflict with the PR which adds the http_session stub file.

The `parse_{cookies,headers,query_params}` methods were added when the
subclass of `requests.Session` was implemented in order to support
setting cookies, headers and query parameters via `k1=v1;k2=v2` strings
(in addition to key-value dicts) via the session API and via the CLI:
- 936e66d
- c6e54fd

Since these methods implement logic purely for the `Streamlink` session
interface and are not meant to be called by any plugin or stream
implementations which use the session's `HTTPSession` instance, they
should be removed. Cookies, headers and query string parameters should
be set directly on their respective `HTTPSession` attributes:
- `cookies`: instance of `requests.cookies.RequestsCookieJar`
- `headers`: instance of `requests.structures.CaseInsensitiveDict`
- `params`: instance of `dict`

Also, at least in regards to HTTP headers, the `key=value` syntax
does not reflect the syntax of raw HTTP requests/responses or interfaces
of other tools like cURL, etc., so having these methods on the
`HTTPSession` class makes it unnecessarily confusing. The method names
themselves are also confusing, as they suggest that the input gets
parsed and that some result gets returned, which is wrong.

This commit therefore moves the `k1=v1;k2=v2` string logic from the
`http_session` module to the `session` module where it belongs and it
also simplifies the option setter.
@bastimeyer bastimeyer force-pushed the plugin/api/http_session/remove-parse-methods branch from 412b1c3 to 3392416 Compare September 5, 2022 13:33
@gravyboat gravyboat merged commit 89d9453 into streamlink:master Sep 5, 2022
@bastimeyer bastimeyer deleted the plugin/api/http_session/remove-parse-methods branch September 5, 2022 18:33
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Sep 15, 2022
The `parse_{cookies,headers,query_params}` methods were added when the
subclass of `requests.Session` was implemented in order to support
setting cookies, headers and query parameters via `k1=v1;k2=v2` strings
(in addition to key-value dicts) via the session API and via the CLI:
- 936e66d
- c6e54fd

Since these methods implement logic purely for the `Streamlink` session
interface and are not meant to be called by any plugin or stream
implementations which use the session's `HTTPSession` instance, they
should be removed. Cookies, headers and query string parameters should
be set directly on their respective `HTTPSession` attributes:
- `cookies`: instance of `requests.cookies.RequestsCookieJar`
- `headers`: instance of `requests.structures.CaseInsensitiveDict`
- `params`: instance of `dict`

Also, at least in regards to HTTP headers, the `key=value` syntax
does not reflect the syntax of raw HTTP requests/responses or interfaces
of other tools like cURL, etc., so having these methods on the
`HTTPSession` class makes it unnecessarily confusing. The method names
themselves are also confusing, as they suggest that the input gets
parsed and that some result gets returned, which is wrong.

This commit therefore moves the `k1=v1;k2=v2` string logic from the
`http_session` module to the `session` module where it belongs and it
also simplifies the option setter.
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.

None yet

2 participants