cli: remove the https-proxy option in favour of a single http-proxy option #4120
Conversation
beardypig
force-pushed
the
4094-drop-https-proxy
branch
2 times, most recently
from
ebd85ae to
fd71fd5
Compare
There was a problem hiding this comment.
The question is, do we want to deprecate the https-proxy option/argument, or do we want to remove it?
The next release will be a major version bump, so removing it can be an option. If we deprecate the option, then we should keep fallback behavior in both streamlink and streamlink_cli. The docs nowadays also have a deprecations page, so newly deprecated stuff should be added there.
You have currently removed logic from streamlink but kept fallback behavior in streamlink_cli, which means the --https-proxy CLI argument will get translated, but session.set_option("https-proxy", value) will not (test_https_proxy_no_effect).
|
Maybe it is better to remove |
|
I think it'd be better to deprecate the option/argument first. Then we don't introduce a disruptive sudden change. Just suppress the CLI argument, keep the logic in streamlink_cli (which sets the We can then remove it with the next major bump (probably when py36 support gets removed, which sees its EoL at the end of this year). |
beardypig
force-pushed
the
4094-drop-https-proxy
branch
2 times, most recently
from
05d5aa7 to
3387bc4
Compare
There was a problem hiding this comment.
We should be very clear that --http-proxy affects HTTP/HTTPS requests + WebSocket connections (for all kinds of supported proxies), and that it's not about proxies via HTTP/HTTPS.
The CLI docs, the --http-proxy argument help text, the http-proxy session option description and deprecation text all differ and are a bit vague, which is not good. This makes it confusing for the end users, who can misinterpret the option.
beardypig
force-pushed
the
4094-drop-https-proxy
branch
from
3387bc4 to
506a609
Compare
beardypig
force-pushed
the
4094-drop-https-proxy
branch
from
506a609 to
ff643e5
Compare
There was a problem hiding this comment.
Thanks. I think this is looking good now. 👍
Let's wait a bit for more feedback though, in case we've missed something.
-
--http-proxyand--https-proxyCLI args now achieve the same thing - Same thing for when using the Session API directly
- Setting the
https-proxyoption prints a deprecation message -
--https-proxydoesn't get listed in the docs / man page / help output anymore - Docs are in good shape
I have made the
--https-proxycommand line argument in to an alias for--http-proxyand suppressed the help output for--https-proxy.https-proxyhas been removed from theget/set_optionmethods, and it will have no effect if set. Aninfolog level message is auto emitted if the--https-proxyoption is used.resolves #4094