From 8e47d2a9a0e13659e15ea2c449068eac4246bf4d Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 27 Jan 2022 20:36:34 -0700 Subject: [PATCH] Removing spread_channel_downloads function in favor of DownloadManager managing next_download. --- modules/videos/__init__.py | 7 --- modules/videos/channel/lib.py | 27 +--------- modules/videos/channel/test/test_api.py | 3 +- modules/videos/conftest.py | 9 ++-- modules/videos/models.py | 13 ++++- modules/videos/test/test_downloader.py | 53 +------------------ modules/videos/test/test_lib.py | 69 +------------------------ 7 files changed, 21 insertions(+), 160 deletions(-) diff --git a/modules/videos/__init__.py b/modules/videos/__init__.py index 5cd95bbb..8bfe14d6 100644 --- a/modules/videos/__init__.py +++ b/modules/videos/__init__.py @@ -1,8 +1 @@ from modules.videos import downloader -from wrolpi import before_startup - - -@before_startup -def startup_spread_channel_downloads(): - from .channel import lib as channel_lib - channel_lib.spread_channel_downloads() diff --git a/modules/videos/channel/lib.py b/modules/videos/channel/lib.py index 64e329ec..d8e49790 100644 --- a/modules/videos/channel/lib.py +++ b/modules/videos/channel/lib.py @@ -115,33 +115,11 @@ def _spread_by_frequency(channels: List[Channel]) -> Generator[Dict, None, None] yield dict(url=channel.url, frequency=frequency, next_download=next_download) -def spread_channel_downloads(): - """ - Channels should be downloaded in a manner that is spread out over their frequency. For example, three channels - with a frequency of a week should be downloaded on different days that week. - """ - with get_db_session(commit=True) as session: - channels = list(session.query(Channel).filter( - Channel.url != None, - Channel.url != '', - Channel.download_frequency != None - ).order_by(Channel.link).all()) # noqa - - url_next_download = _spread_by_frequency(channels) - - for info in url_next_download: - url, frequency, next_download = info['url'], info['frequency'], info['next_download'] - download = download_manager.get_or_create_download(url, session) - download.downloader = 'video_channel' - download.frequency = frequency - download.next_download = next_download - - @run_after(save_channels_config) def update_channel(data, link): with get_db_session(commit=True) as session: try: - channel = session.query(Channel).filter_by(link=link).one() + channel: Channel = session.query(Channel).filter_by(link=link).one() except NoResultFound: raise UnknownChannel() @@ -184,13 +162,10 @@ def update_channel(data, link): # Apply the changes now that we've OK'd them channel.update(data) - spread_channel_downloads() - return channel @run_after(save_channels_config) -@run_after(spread_channel_downloads) def create_channel(data: dict, return_dict: bool = True) -> Union[Channel, dict]: """ Create a new Channel. Check for conflicts with existing Channels. diff --git a/modules/videos/channel/test/test_api.py b/modules/videos/channel/test/test_api.py index a610bb92..6e30d309 100644 --- a/modules/videos/channel/test/test_api.py +++ b/modules/videos/channel/test/test_api.py @@ -10,7 +10,7 @@ import mock import pytest from modules.videos.api import refresh_queue -from modules.videos.channel.lib import spread_channel_downloads, delete_channel +from modules.videos.channel.lib import delete_channel from modules.videos.common import get_no_channel_directory from modules.videos.lib import upsert_video from modules.videos.models import Channel, Video @@ -380,7 +380,6 @@ class TestVideoAPI(TestAPI): location = response.headers['Location'] # No downloads are scheduled. - spread_channel_downloads() with get_db_session() as session: downloads = list(download_manager.get_new_downloads(session)) self.assertEqual(len(downloads), 0) diff --git a/modules/videos/conftest.py b/modules/videos/conftest.py index c4762c1c..b25edee4 100644 --- a/modules/videos/conftest.py +++ b/modules/videos/conftest.py @@ -7,11 +7,10 @@ import mock import pytest from PIL import Image -from modules.videos.channel.lib import spread_channel_downloads from modules.videos.downloader import VideoDownloader, ChannelDownloader from modules.videos.models import Channel, Video from wrolpi.common import sanitize_link -from wrolpi.downloader import DownloadFrequency, DownloadManager +from wrolpi.downloader import DownloadFrequency, DownloadManager, Download from wrolpi.vars import PROJECT_DIR @@ -57,7 +56,7 @@ def channel_factory(test_session, test_directory): @pytest.fixture -def download_channel(test_session, test_directory) -> Channel: +def download_channel(test_session, test_directory, video_download_manager) -> Channel: """ Get a test Channel that has a download frequency. """ @@ -69,9 +68,9 @@ def download_channel(test_session, test_directory) -> Channel: download_frequency=DownloadFrequency.weekly, link='downloadchannel', ) - test_session.add(channel) + download = Download(url=channel.url, downloader='video_channel', frequency=channel.download_frequency) + test_session.add_all([channel, download]) test_session.commit() - spread_channel_downloads() return channel diff --git a/modules/videos/models.py b/modules/videos/models.py index b9ca0190..68ca22c1 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -11,7 +11,7 @@ from sqlalchemy.orm.collections import InstrumentedList from wrolpi.common import Base, tsvector, ModelHelper, logger, get_media_directory from wrolpi.dates import now, TZDateTime from wrolpi.db import get_db_curs -from wrolpi.downloader import Download +from wrolpi.downloader import Download, download_manager from wrolpi.errors import UnknownVideo, UnknownFile, UnknownDirectory from wrolpi.media_path import MediaPathType, MediaPath @@ -302,9 +302,20 @@ class Channel(ModelHelper, Base): session.delete(self) def update(self, data: dict): + """ + Update the values of this Channel. Will also update the Channel's Download, if it has one. + """ for key, value in data.items(): setattr(self, key, value) + if not self.url: + return + download = self.get_download() + if download: + session = Session.object_session(self) + download.frequency = self.download_frequency + download.next_download = download_manager.get_next_download(download, session) + def config_view(self) -> dict: """ Retrieve the data about this Channel that should be stored in a config file. diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index 7aa2d90b..60684bde 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -1,19 +1,17 @@ import pathlib from copy import copy -from datetime import datetime from unittest import mock import pytest from yt_dlp.utils import UnsupportedError -from modules.videos.channel.lib import spread_channel_downloads, download_channel +from modules.videos.channel.lib import download_channel from modules.videos.downloader import find_all_missing_videos, VideoDownloader, \ ChannelDownloader, get_or_create_channel from modules.videos.models import Channel, Video from modules.videos.test.common import create_channel_structure -from wrolpi.dates import local_timezone from wrolpi.db import get_db_session, get_db_context -from wrolpi.downloader import DownloadManager, Download, DownloadFrequency +from wrolpi.downloader import DownloadManager, Download from wrolpi.errors import InvalidDownload from wrolpi.test.common import wrap_test_db, TestAPI from wrolpi.test.test_downloader import HTTPDownloader @@ -286,53 +284,6 @@ class TestVideosDownloaders(TestAPI): download = self.mgr.get_downloads(session)[0] assert download.status == 'failed' - @wrap_test_db - @mock.patch('modules.videos.channel.lib.today', lambda: local_timezone(datetime(2020, 1, 1, 0, 0, 0))) - def test_spread_channel_downloads(self): - with get_db_session(commit=True) as session: - channel1 = Channel(name='channel1', link='channel1', url='https://example.com/1', - download_frequency=DownloadFrequency.weekly) - channel2 = Channel(name='channel2', link='channel2', url='https://example.com/2', - download_frequency=DownloadFrequency.weekly) - session.add_all([channel1, channel2]) - - def check_frequencies(expected): - with get_db_session(commit=True) as session: - downloads = list(session.query(Download).all()) - self.assertLength(downloads, expected) - for download in downloads: - next_download = expected[download.url] - self.assertEqual(next_download, str(download.next_download), - f'{download.url} frequency {next_download} != {download.next_download}') - - # Spread the downloads out. - spread_channel_downloads() - - expected = { - 'https://example.com/1': '2020-01-08 00:00:00-07:00', - 'https://example.com/2': '2020-01-11 12:00:00-07:00', - } - check_frequencies(expected) - - with get_db_session(commit=True) as session: - channel3 = Channel(name='channel3', link='channel3', url='https://example.com/3', - download_frequency=DownloadFrequency.daily) - channel4 = Channel(name='channel4', link='channel4', url='https://example.com/4', - download_frequency=DownloadFrequency.weekly) - # 5 and 6 should be ignored. - channel5 = Channel(name='channel5', link='channel5', url='https://example.com/5') - channel6 = Channel(name='channel6', link='channel6', download_frequency=DownloadFrequency.daily) - session.add_all([channel3, channel4, channel5, channel6]) - - spread_channel_downloads() - expected = { - 'https://example.com/1': '2020-01-08 00:00:00-07:00', - 'https://example.com/2': '2020-01-10 08:00:00-07:00', - 'https://example.com/3': '2020-01-02 00:00:00-07:00', - 'https://example.com/4': '2020-01-12 16:00:00-07:00', - } - check_frequencies(expected) - @wrap_test_db def test_get_or_create_channel(self): """ diff --git a/modules/videos/test/test_lib.py b/modules/videos/test/test_lib.py index b16a66f5..bdf35d3b 100644 --- a/modules/videos/test/test_lib.py +++ b/modules/videos/test/test_lib.py @@ -7,80 +7,13 @@ from unittest import mock import pytest import sqlalchemy -from modules.videos.channel import lib from modules.videos.common import apply_info_json from modules.videos.lib import validate_videos, parse_video_file_name, upsert_video -from modules.videos.models import Channel, Video +from modules.videos.models import Video from modules.videos.video.lib import video_search from wrolpi.dates import local_timezone -from wrolpi.downloader import DownloadFrequency from wrolpi.vars import PROJECT_DIR -daily, weekly, days30 = DownloadFrequency.daily, DownloadFrequency.weekly, DownloadFrequency.days30 - - -@pytest.mark.parametrize( - 'channels,expected', - [ - ([Channel(download_frequency=daily, url='https://example.com/1')], - [('https://example.com/1', daily, '2000-01-02 00:00:00-07:00')] - ), - # Several different download frequencies. - ([ - Channel(download_frequency=daily, url='https://example.com/1'), - Channel(download_frequency=daily, url='https://example.com/2'), - Channel(download_frequency=daily, url='https://example.com/3'), - Channel(download_frequency=weekly, url='https://example.com/4'), - Channel(download_frequency=weekly, url='https://example.com/5'), - Channel(download_frequency=days30, url='https://example.com/6'), - ], - [ - ('https://example.com/1', daily, '2000-01-02 00:00:00-07:00'), - ('https://example.com/2', daily, '2000-01-02 08:00:00-07:00'), - ('https://example.com/3', daily, '2000-01-02 16:00:00-07:00'), - ('https://example.com/4', weekly, '2000-01-08 00:00:00-07:00'), - ('https://example.com/5', weekly, '2000-01-11 12:00:00-07:00'), - ('https://example.com/6', days30, '2000-01-31 00:00:00-07:00'), - ] - ), - # Multiple downloads in a day. - ([Channel(download_frequency=daily, url=f'https://example.com/{i}') for i in range(1, 3)], - [ - ('https://example.com/1', daily, '2000-01-02 00:00:00-07:00'), - ('https://example.com/2', daily, '2000-01-02 12:00:00-07:00'), - ] - ), - # More downloads in a week than days in a week. - ([Channel(download_frequency=weekly, url=f'https://example.com/{i}') for i in range(1, 9)], - [ - ('https://example.com/1', weekly, '2000-01-08 00:00:00-07:00'), - ('https://example.com/2', weekly, '2000-01-08 21:00:00-07:00'), - ('https://example.com/3', weekly, '2000-01-09 18:00:00-07:00'), - ('https://example.com/4', weekly, '2000-01-10 15:00:00-07:00'), - ('https://example.com/5', weekly, '2000-01-11 12:00:00-07:00'), - ('https://example.com/6', weekly, '2000-01-12 09:00:00-07:00'), - ('https://example.com/7', weekly, '2000-01-13 06:00:00-07:00'), - ('https://example.com/8', weekly, '2000-01-14 03:00:00-07:00'), - ] - ), - # Any Channel without a URL AND frequency is ignored. - ([ - Channel(download_frequency=daily, url='https://example.com/1'), - Channel(download_frequency=daily), - Channel(url='this should be ignored'), - ], - [('https://example.com/1', daily, '2000-01-02 00:00:00-07:00')] - ), - ] -) -@mock.patch('modules.videos.channel.lib.today', lambda: local_timezone(datetime(2000, 1, 1, 0, 0, 0))) -def test_spread_by_frequency(channels, expected): - result = lib._spread_by_frequency(channels) - for result, (expected_url, expected_frequency, expected_dt) in zip(result, expected): - assert result['url'] == expected_url - assert result['frequency'] == expected_frequency - assert str(result['next_download']) == expected_dt - def test_search_censored_videos(test_session, simple_channel): for i in map(str, range(50)):