From 1e3961a9c6108866e5f1ac66b53a7f0d279447d4 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Tue, 29 Nov 2022 23:31:12 -0800 Subject: [PATCH] fix playlist last thumbnail move to copy --- src/ytdl_sub/downloaders/downloader.py | 3 +- src/ytdl_sub/plugins/nfo_tags.py | 3 ++ .../subscriptions/subscription_download.py | 16 ++++++- tests/conftest.py | 26 +++++++++- tests/e2e/conftest.py | 47 +++++++++++++------ tests/e2e/youtube/test_playlist.py | 4 +- 6 files changed, 78 insertions(+), 21 deletions(-) diff --git a/src/ytdl_sub/downloaders/downloader.py b/src/ytdl_sub/downloaders/downloader.py index 4456cdbc..f9712ccf 100644 --- a/src/ytdl_sub/downloaders/downloader.py +++ b/src/ytdl_sub/downloaders/downloader.py @@ -579,9 +579,9 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): "Beginning downloads for %s", self.overrides.apply_formatter(collection_url.url) ) for entry in self._download(parents=parents, orphans=orphan_entries): - yield entry # Update thumbnails in case of last_entry self._download_url_thumbnails(collection_url=collection_url, entry=entry) + yield entry @classmethod def _download_thumbnail( @@ -630,6 +630,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): self.save_file( file_name=entry.get_download_thumbnail_name(), output_file_name=thumbnail_name, + copy_file=True, ) self._url_state.thumbnails_downloaded.add(thumbnail_name) continue diff --git a/src/ytdl_sub/plugins/nfo_tags.py b/src/ytdl_sub/plugins/nfo_tags.py index ffd8c225..4bde7d4c 100644 --- a/src/ytdl_sub/plugins/nfo_tags.py +++ b/src/ytdl_sub/plugins/nfo_tags.py @@ -10,6 +10,7 @@ from typing import Optional from ytdl_sub.entries.entry import Entry from ytdl_sub.plugins.plugin import Plugin from ytdl_sub.plugins.plugin import PluginOptions +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileMetadata from ytdl_sub.utils.xml import XmlElement from ytdl_sub.utils.xml import to_max_3_byte_utf8_dict @@ -171,6 +172,8 @@ class SharedNfoTagsPlugin(Plugin[SharedNfoTagsOptions], ABC): else: self.save_file(file_name=nfo_file_name, file_metadata=nfo_metadata) + FileHandler.delete(nfo_file_path) + class NfoTagsOptions(SharedNfoTagsOptions): """ diff --git a/src/ytdl_sub/subscriptions/subscription_download.py b/src/ytdl_sub/subscriptions/subscription_download.py index 380fd155..38424792 100644 --- a/src/ytdl_sub/subscriptions/subscription_download.py +++ b/src/ytdl_sub/subscriptions/subscription_download.py @@ -11,6 +11,7 @@ from ytdl_sub.subscriptions.base_subscription import BaseSubscription from ytdl_sub.subscriptions.subscription_ytdl_options import SubscriptionYTDLOptions from ytdl_sub.utils.datetime import to_date_range from ytdl_sub.utils.exceptions import ValidationException +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileHandlerTransactionLog from ytdl_sub.utils.file_handler import FileMetadata from ytdl_sub.utils.thumbnail import convert_download_thumbnail @@ -94,6 +95,10 @@ class SubscriptionDownload(BaseSubscription, ABC): entry=entry, ) + def _delete_working_directory(self, is_error: bool) -> None: + _ = is_error + shutil.rmtree(self.working_directory) + @contextlib.contextmanager def _prepare_working_directory(self): """ @@ -104,8 +109,11 @@ class SubscriptionDownload(BaseSubscription, ABC): try: yield - finally: - shutil.rmtree(self.working_directory) + except Exception as exc: + self._delete_working_directory(is_error=True) + raise exc + else: + self._delete_working_directory(is_error=False) @contextlib.contextmanager def _maintain_archive_file(self): @@ -268,6 +276,10 @@ class SubscriptionDownload(BaseSubscription, ABC): plugins=plugins, dry_run=dry_run, entry=entry, entry_metadata=entry_metadata ) + FileHandler.delete(entry.get_download_file_path()) + FileHandler.delete(entry.get_download_thumbnail_path()) + FileHandler.delete(entry.get_download_info_json_path()) + for plugin in plugins: plugin.post_process_subscription() diff --git a/tests/conftest.py b/tests/conftest.py index 7e863503..4b94bbef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,6 @@ import json import logging import shutil import tempfile -from pathlib import Path from typing import Any from typing import Callable from typing import Dict @@ -11,9 +10,13 @@ from typing import List from unittest.mock import patch import pytest +from expected_download import _get_files_in_directory +from ytdl_sub.subscriptions.subscription_download import SubscriptionDownload from ytdl_sub.utils.logger import Logger +logger = Logger.get("test") + @pytest.fixture() def output_directory(): @@ -23,8 +26,27 @@ def output_directory(): @pytest.fixture def working_directory() -> str: + """ + Any time the working directory is used, ensure no files remain on cleaning it up + """ with tempfile.TemporaryDirectory() as temp_dir: - yield temp_dir + + def _assert_working_directory_empty(self, is_error: bool): + files = [str(file_path) for file_path in _get_files_in_directory(temp_dir)] + num_files = len(files) + shutil.rmtree(temp_dir) + + if not is_error: + if num_files > 0: + logger.error("left-over files in working dir:\n%s", "\n".join(files)) + assert num_files == 0 + + with patch.object( + SubscriptionDownload, + "_delete_working_directory", + new=_assert_working_directory_empty, + ): + yield temp_dir @contextlib.contextmanager diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index b12160e8..3604f775 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -1,3 +1,4 @@ +import json import sys import tempfile from typing import List @@ -10,6 +11,7 @@ from ytdl_sub.cli.main import main from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.subscriptions.subscription import Subscription from ytdl_sub.utils.file_handler import FileHandlerTransactionLog +from ytdl_sub.utils.yaml import load_yaml @pytest.fixture() @@ -17,29 +19,46 @@ def music_video_config_path(): return "examples/music_videos_config.yaml" -@pytest.fixture() -def music_video_config(music_video_config_path): - return ConfigFile.from_file_path(config_path=music_video_config_path) +def _load_config(config_path: str, working_directory: str) -> ConfigFile: + config_dict = load_yaml(file_path=config_path) + config_dict["configuration"]["working_directory"] = working_directory + + return ConfigFile.from_dict(config_dict) @pytest.fixture() -def channel_as_tv_show_config(): - return ConfigFile.from_file_path(config_path="examples/tv_show_config.yaml") +def music_video_config(music_video_config_path, working_directory) -> ConfigFile: + return _load_config(music_video_config_path, working_directory) + + +@pytest.fixture() +def music_video_config_for_cli(music_video_config) -> str: + with tempfile.NamedTemporaryFile(suffix=".yaml") as tmp_file: + tmp_file.write(json.dumps(music_video_config._value).encode("utf-8")) + tmp_file.flush() + yield tmp_file.name + + +@pytest.fixture() +def channel_as_tv_show_config(working_directory) -> ConfigFile: + return _load_config( + config_path="examples/tv_show_config.yaml", working_directory=working_directory + ) @pytest.fixture -def soundcloud_discography_config(): - return ConfigFile.from_file_path(config_path="examples/soundcloud_discography_config.yaml") +def soundcloud_discography_config(working_directory) -> ConfigFile: + return _load_config( + config_path="examples/soundcloud_discography_config.yaml", + working_directory=working_directory, + ) @pytest.fixture() -def youtube_audio_config_path(): - return "examples/music_audio_from_videos.yaml" - - -@pytest.fixture() -def youtube_audio_config(youtube_audio_config_path): - return ConfigFile.from_file_path(config_path=youtube_audio_config_path) +def youtube_audio_config(working_directory) -> ConfigFile: + return _load_config( + config_path="examples/music_audio_from_videos.yaml", working_directory=working_directory + ) @pytest.fixture diff --git a/tests/e2e/youtube/test_playlist.py b/tests/e2e/youtube/test_playlist.py index 86265bd0..658d5872 100644 --- a/tests/e2e/youtube/test_playlist.py +++ b/tests/e2e/youtube/test_playlist.py @@ -97,7 +97,7 @@ class TestPlaylist: def test_playlist_download_from_cli_sub( self, preset_dict_to_subscription_yaml_generator, - music_video_config_path, + music_video_config_for_cli, playlist_preset_dict, output_directory, dry_run, @@ -106,7 +106,7 @@ class TestPlaylist: subscription_name="music_video_playlist_test", preset_dict=playlist_preset_dict ) as subscription_path: args = "--dry-run " if dry_run else "" - args += f"--config {music_video_config_path} " + args += f"--config {music_video_config_for_cli} " args += f"sub {subscription_path}" subscription_transaction_log = mock_run_from_cli(args=args)