forked from DRMTalks/devine
Remove Track.needs_proxy
Ok, so there's a few reasons this was done. 1) Design-wise it isn't valid to have --proxy (or via config/otherwise) set a proxy, then unpredictably have it bypassed or disabled. If I specify `--proxy 127.0.0.1:8080`, I would expect it to use that proxy for all communication indefinitely, not switch in and out depending on the track or service. 2) With reason 1, it's also a security problem. The only reason I implemented it in the first place was so I could download faster on my home connection. This means I would authenticate and call APIs under a proxy, then suddenly download manifests and segments e.t.c under my home connection. A competent service could see that as an indicator of bad play and flag you. 3) Maintaining this setup across the codebase is extremely annoying, especially because of how proxies are setup/used by Requests in the Session. There's no way to tell a request session to temporarily disable the proxy and turn it back on later, without having to get the proxy from the session (in an annoying way) store it, then remove it, make the calls, then assuming your still in the same function you can add it back. If you're not in the same function, well, time for some spaghetti code. --- tldr; -1 ux/design/expectations with CLI, -1 security aspect, -1 code maintenance, but only +1 for potentially increased download speeds in certain scenarios.
This commit is contained in:
parent
3c1c408ccd
commit
c0d940b17b
|
@ -831,10 +831,7 @@ class dl:
|
|||
progress(downloaded="[yellow]SKIPPED")
|
||||
return
|
||||
|
||||
if track.needs_proxy:
|
||||
proxy = next(iter(service.session.proxies.values()), None)
|
||||
else:
|
||||
proxy = None
|
||||
|
||||
save_path = config.directories.temp / f"{track.__class__.__name__}_{track.id}.mp4"
|
||||
if isinstance(track, Subtitle):
|
||||
|
@ -925,7 +922,7 @@ class dl:
|
|||
out=save_path,
|
||||
headers=service.session.headers,
|
||||
cookies=service.session.cookies,
|
||||
proxy=proxy if track.needs_proxy else None,
|
||||
proxy=proxy,
|
||||
progress=progress
|
||||
)
|
||||
|
||||
|
@ -964,14 +961,7 @@ class dl:
|
|||
|
||||
if not self.DL_POOL_SKIP.is_set():
|
||||
if track.path.stat().st_size <= 3: # Empty UTF-8 BOM == 3 bytes
|
||||
raise IOError(
|
||||
"Download failed, the downloaded file is empty. "
|
||||
f"This {'was' if track.needs_proxy else 'was not'} downloaded with a proxy." +
|
||||
(
|
||||
" Perhaps you need to set `needs_proxy` as True to use the proxy for this track."
|
||||
if not track.needs_proxy else ""
|
||||
)
|
||||
)
|
||||
raise IOError("Download failed, the downloaded file is empty.")
|
||||
|
||||
if callable(track.OnDownloaded):
|
||||
track.OnDownloaded(track)
|
||||
|
|
|
@ -237,9 +237,6 @@ class DASH:
|
|||
elif not isinstance(session, Session):
|
||||
raise TypeError(f"Expected session to be a {Session}, not {session!r}")
|
||||
|
||||
if not track.needs_proxy and proxy:
|
||||
proxy = None
|
||||
|
||||
if proxy:
|
||||
session.proxies.update({
|
||||
"all": proxy
|
||||
|
|
|
@ -202,9 +202,6 @@ class HLS:
|
|||
elif not isinstance(session, Session):
|
||||
raise TypeError(f"Expected session to be a {Session}, not {session!r}")
|
||||
|
||||
if not track.needs_proxy and proxy:
|
||||
proxy = None
|
||||
|
||||
if proxy:
|
||||
session.proxies.update({
|
||||
"all": proxy
|
||||
|
|
|
@ -32,7 +32,6 @@ class Track:
|
|||
language: Union[Language, str],
|
||||
is_original_lang: bool = False,
|
||||
descriptor: Descriptor = Descriptor.URL,
|
||||
needs_proxy: bool = False,
|
||||
needs_repack: bool = False,
|
||||
drm: Optional[Iterable[DRM_T]] = None,
|
||||
edition: Optional[str] = None,
|
||||
|
@ -45,7 +44,6 @@ class Track:
|
|||
self.is_original_lang = bool(is_original_lang)
|
||||
# optional io metadata
|
||||
self.descriptor = descriptor
|
||||
self.needs_proxy = bool(needs_proxy)
|
||||
self.needs_repack = bool(needs_repack)
|
||||
# drm
|
||||
self.drm = drm
|
||||
|
|
Loading…
Reference in New Issue