From 9fd97beca32cb17352916cb9c94ba50f45d0ac87 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Mon, 1 Aug 2022 12:39:23 -0700 Subject: [PATCH] [BACKEND|BUG] Retry downloads with missing files (#137) * [BACKEND|BUG] Retry downloads with missing files * fix if statement, rename channel test file --- src/ytdl_sub/downloaders/downloader.py | 66 ++++++++++++++++++- .../soundcloud/albums_and_singles.py | 9 ++- src/ytdl_sub/downloaders/youtube/channel.py | 3 +- src/ytdl_sub/downloaders/youtube/playlist.py | 3 +- src/ytdl_sub/entries/entry.py | 35 ++++++++++ src/ytdl_sub/utils/exceptions.py | 4 ++ src/ytdl_sub/utils/thumbnail.py | 22 +------ ...nel_as_kodi_tv_show.py => test_channel.py} | 0 8 files changed, 114 insertions(+), 28 deletions(-) rename tests/e2e/youtube/{test_channel_as_kodi_tv_show.py => test_channel.py} (100%) diff --git a/src/ytdl_sub/downloaders/downloader.py b/src/ytdl_sub/downloaders/downloader.py index 740e56ab..eb73b43c 100644 --- a/src/ytdl_sub/downloaders/downloader.py +++ b/src/ytdl_sub/downloaders/downloader.py @@ -1,10 +1,13 @@ import abc import contextlib +import copy import json import os +import time from abc import ABC from contextlib import contextmanager from pathlib import Path +from typing import Callable from typing import Dict from typing import Generic from typing import Iterable @@ -22,6 +25,7 @@ from ytdl_sub.config.preset_options import Overrides from ytdl_sub.entries.base_entry import BaseEntry from ytdl_sub.entries.entry import Entry from ytdl_sub.thread.log_entries_downloaded_listener import LogEntriesDownloadedListener +from ytdl_sub.utils.exceptions import FileNotDownloadedException from ytdl_sub.utils.file_handler import FileMetadata from ytdl_sub.utils.logger import Logger from ytdl_sub.validators.strict_dict_validator import StrictDictValidator @@ -53,6 +57,9 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] supports_download_archive: bool = True + _extract_entry_num_retries: int = 5 + _extract_entry_retry_wait_sec: int = 1 + @classmethod def ytdl_option_overrides(cls) -> Dict: """Global overrides that even overwrite user input""" @@ -121,6 +128,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] if ytdl_options_overrides is not None: ytdl_options = dict(ytdl_options, **ytdl_options_overrides) + download_logger.debug("ytdl_options: %s", str(ytdl_options)) with Logger.handle_external_logs(name="yt-dlp"): with ytdl.YoutubeDL(ytdl_options) as ytdl_downloader: yield ytdl_downloader @@ -145,11 +153,65 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] Optional. Dict containing ytdl args to override other predefined ytdl args **kwargs arguments passed directory to YoutubeDL extract_info - """ with self.ytdl_downloader(ytdl_options_overrides) as ytdl_downloader: return ytdl_downloader.extract_info(**kwargs) + def extract_info_with_retry( + self, + is_downloaded_fn: Callable[[], bool], + ytdl_options_overrides: Optional[Dict] = None, + **kwargs, + ) -> Dict: + """ + Wrapper around yt_dlp.YoutubeDL.YoutubeDL.extract_info + All kwargs will passed to the extract_info function. + + This should be used when downloading a single entry. Checks if the entry's video + and thumbnail files exist - retry if they do not. + + Parameters + ---------- + is_downloaded_fn + Function to check if the entry is downloaded + ytdl_options_overrides + Optional. Dict containing ytdl args to override other predefined ytdl args + **kwargs + arguments passed directory to YoutubeDL extract_info + + Raises + ------ + FileNotDownloadedException + If the entry fails to download + """ + num_tries = 0 + entry_files_exist = False + ytdl_options_overrides = copy.deepcopy(ytdl_options_overrides) + + while not entry_files_exist and num_tries < self._extract_entry_num_retries: + entry_dict = self.extract_info(ytdl_options_overrides=ytdl_options_overrides, **kwargs) + if is_downloaded_fn(): + return entry_dict + + time.sleep(self._extract_entry_retry_wait_sec) + num_tries += 1 + + # Remove the download archive so it can retry without thinking its already downloaded, + # even though it is not + ytdl_options_overrides["download_archive"] = None + + if num_tries < self._extract_entry_retry_wait_sec: + download_logger.debug( + "Failed to download entry. Retrying %d / %d", + num_tries, + self._extract_entry_num_retries, + ) + + error_dict = {"ytdl_options": ytdl_options_overrides, "kwargs": kwargs} + raise FileNotDownloadedException( + f"yt-dlp failed to download an entry with these arguments: {error_dict}" + ) + def _get_entry_dicts_from_info_json_files(self) -> List[Dict]: """ Returns @@ -197,7 +259,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] ytdl_options_overrides: Optional[Dict] = None, only_info_json: bool = False, log_prefix_on_info_json_dl: Optional[str] = None, - **kwargs + **kwargs, ) -> List[Dict]: """ Wrapper around yt_dlp.YoutubeDL.YoutubeDL.extract_info with infojson enabled. Entry dicts diff --git a/src/ytdl_sub/downloaders/soundcloud/albums_and_singles.py b/src/ytdl_sub/downloaders/soundcloud/albums_and_singles.py index f3a044b6..16bd31e9 100644 --- a/src/ytdl_sub/downloaders/soundcloud/albums_and_singles.py +++ b/src/ytdl_sub/downloaders/soundcloud/albums_and_singles.py @@ -146,7 +146,8 @@ class SoundcloudAlbumsAndSinglesDownloader( track.title, ) if not self.is_dry_run: - _ = self.extract_info( + _ = self.extract_info_with_retry( + is_downloaded_fn=track.is_downloaded, url=album.kwargs("webpage_url"), ytdl_options_overrides={ "playlist_items": str(track.kwargs("playlist_index")), @@ -175,8 +176,10 @@ class SoundcloudAlbumsAndSinglesDownloader( download_logger.info("Downloading single track %s", track.title) if not self.is_dry_run: - _ = self.extract_info( - url=track.kwargs("webpage_url"), ytdl_options_overrides={"writeinfojson": False} + _ = self.extract_info_with_retry( + is_downloaded_fn=track.is_downloaded, + url=track.kwargs("webpage_url"), + ytdl_options_overrides={"writeinfojson": False}, ) yield track diff --git a/src/ytdl_sub/downloaders/youtube/channel.py b/src/ytdl_sub/downloaders/youtube/channel.py index ab8b473e..2fc6f14e 100644 --- a/src/ytdl_sub/downloaders/youtube/channel.py +++ b/src/ytdl_sub/downloaders/youtube/channel.py @@ -163,7 +163,8 @@ class YoutubeChannelDownloader(YoutubeDownloader[YoutubeChannelDownloaderOptions # Only do the individual download if it is not dry-run if not self.is_dry_run: - _ = self.extract_info( + _ = self.extract_info_with_retry( + is_downloaded_fn=video.is_downloaded, ytdl_options_overrides={ "playlist_items": str(video.kwargs("playlist_index")), "writeinfojson": False, diff --git a/src/ytdl_sub/downloaders/youtube/playlist.py b/src/ytdl_sub/downloaders/youtube/playlist.py index 24c64674..dce80fa3 100644 --- a/src/ytdl_sub/downloaders/youtube/playlist.py +++ b/src/ytdl_sub/downloaders/youtube/playlist.py @@ -90,7 +90,8 @@ class YoutubePlaylistDownloader( # Only do the individual download if it is not dry-run and downloading individually if not self.is_dry_run: - _ = self.extract_info( + _ = self.extract_info_with_retry( + is_downloaded_fn=video.is_downloaded, ytdl_options_overrides={ "playlist_items": str(entry_dict.get("playlist_index")), "writeinfojson": False, diff --git a/src/ytdl_sub/entries/entry.py b/src/ytdl_sub/entries/entry.py index 7613a928..5970bc26 100644 --- a/src/ytdl_sub/entries/entry.py +++ b/src/ytdl_sub/entries/entry.py @@ -1,5 +1,7 @@ +import os from pathlib import Path from typing import Dict +from typing import Optional from typing import final from ytdl_sub.entries.base_entry import BaseEntry @@ -38,6 +40,39 @@ class Entry(EntryVariables, BaseEntry): """Returns the entry's thumbnail's file path to where it was downloaded""" return str(Path(self.working_directory()) / self.get_download_thumbnail_name()) + def get_ytdlp_download_thumbnail_path(self) -> Optional[str]: + """ + The source `thumbnail` value and the actual downloaded thumbnail extension sometimes do + not match. Return the actual downloaded thumbnail path. + """ + thumbnails = self.kwargs("thumbnails") or [] + possible_thumbnail_exts = {"jpg", "webp"} # Always check for jpg and webp thumbs + + for thumbnail in thumbnails: + possible_thumbnail_exts.add(thumbnail["url"].split(".")[-1]) + + for ext in possible_thumbnail_exts: + possible_thumbnail_path = str(Path(self.working_directory()) / f"{self.uid}.{ext}") + if os.path.isfile(possible_thumbnail_path): + return possible_thumbnail_path + + return None + + @final + def is_downloaded(self) -> bool: + """ + Returns + ------- + True if the file and thumbnail exist locally. False otherwise. + """ + thumbnail_exists = ( + os.path.isfile(self.get_download_thumbnail_path()) + or self.get_ytdlp_download_thumbnail_path() is not None + ) + file_exists = os.path.isfile(self.get_download_file_path()) + + return file_exists and thumbnail_exists + @final def to_dict(self) -> Dict[str, str]: """ diff --git a/src/ytdl_sub/utils/exceptions.py b/src/ytdl_sub/utils/exceptions.py index 6798c763..1a545b69 100644 --- a/src/ytdl_sub/utils/exceptions.py +++ b/src/ytdl_sub/utils/exceptions.py @@ -32,3 +32,7 @@ class RegexNoMatchException(ValidationException): class InvalidDlArguments(ValidationException): """dl arguments that are invalid""" + + +class FileNotDownloadedException(ValueError): + """ytdlp failed to download something""" diff --git a/src/ytdl_sub/utils/thumbnail.py b/src/ytdl_sub/utils/thumbnail.py index 0a3d4b93..df56f3f5 100644 --- a/src/ytdl_sub/utils/thumbnail.py +++ b/src/ytdl_sub/utils/thumbnail.py @@ -1,30 +1,10 @@ -import os import tempfile -from pathlib import Path -from typing import Optional from urllib.request import urlopen from ytdl_sub.entries.entry import Entry from ytdl_sub.utils.ffmpeg import FFMPEG -def _get_downloaded_thumbnail_path(entry: Entry) -> Optional[str]: - thumbnails = entry.kwargs("thumbnails") or [] - possible_thumbnail_exts = {"jpg", "webp"} # Always check for jpg and webp thumbs - - # The source `thumbnail` value and the actual downloaded thumbnail extension sometimes do - # not match. Find all possible extensions by checking all available thumbnails. - for thumbnail in thumbnails: - possible_thumbnail_exts.add(thumbnail["url"].split(".")[-1]) - - for ext in possible_thumbnail_exts: - possible_thumbnail_path = str(Path(entry.working_directory()) / f"{entry.uid}.{ext}") - if os.path.isfile(possible_thumbnail_path): - return possible_thumbnail_path - - return None - - def convert_download_thumbnail(entry: Entry): """ Converts an entry's downloaded thumbnail into jpg format @@ -39,7 +19,7 @@ def convert_download_thumbnail(entry: Entry): ValueError Entry thumbnail file not found """ - download_thumbnail_path = _get_downloaded_thumbnail_path(entry=entry) + download_thumbnail_path = entry.get_ytdlp_download_thumbnail_path() download_thumbnail_path_as_jpg = entry.get_download_thumbnail_path() if not download_thumbnail_path: raise ValueError("Thumbnail not found") diff --git a/tests/e2e/youtube/test_channel_as_kodi_tv_show.py b/tests/e2e/youtube/test_channel.py similarity index 100% rename from tests/e2e/youtube/test_channel_as_kodi_tv_show.py rename to tests/e2e/youtube/test_channel.py