From b28bacfd5be6b0df82944b246db7248cc4e3c405 Mon Sep 17 00:00:00 2001 From: Roland Date: Tue, 28 Dec 2021 18:17:53 -0700 Subject: [PATCH] Downloader names are in the class, not in the init. --- modules/archive/__init__.py | 4 ++- modules/videos/conftest.py | 4 +-- modules/videos/downloader.py | 6 +++-- modules/videos/test/test_downloader.py | 4 +-- wrolpi/downloader.py | 9 ++++--- wrolpi/test/test_downloader.py | 37 +++++++++++++++++++------- 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/modules/archive/__init__.py b/modules/archive/__init__.py index 89005692..caeebc8e 100644 --- a/modules/archive/__init__.py +++ b/modules/archive/__init__.py @@ -13,6 +13,8 @@ logger = logger.getChild(__name__) class ArchiveDownloader(Downloader, ABC): + name = 'archive' + @classmethod def valid_url(cls, url: str) -> Tuple[bool, None]: """ @@ -29,4 +31,4 @@ class ArchiveDownloader(Downloader, ABC): # Archive downloader is the last downloader which should be used. -archive_downloader = ArchiveDownloader('archive', priority=100) +archive_downloader = ArchiveDownloader(priority=100) diff --git a/modules/videos/conftest.py b/modules/videos/conftest.py index 55363f85..28d5d738 100644 --- a/modules/videos/conftest.py +++ b/modules/videos/conftest.py @@ -114,8 +114,8 @@ def video_download_manager(test_download_manager) -> DownloadManager: """ Get a DownloadManager ready to download Videos and Channels. """ - channel_downloader = ChannelDownloader('video_channel') - video_downloader = VideoDownloader('video', priority=40) + channel_downloader = ChannelDownloader() + video_downloader = VideoDownloader(priority=40) test_download_manager.register_downloader(channel_downloader) test_download_manager.register_downloader(video_downloader) diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index 72583a07..4583a3cd 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -47,6 +47,7 @@ class ChannelDownloader(Downloader, ABC): """ Handling downloading of all Videos in a Channel. """ + name = 'video_channel' @classmethod def valid_url(cls, url) -> tuple[bool, None]: @@ -90,6 +91,7 @@ class VideoDownloader(Downloader, ABC): """ Download a single video. Store the video in it's channel's directory, otherwise store it in `videos/NO CHANNEL`. """ + name = 'video' @classmethod def valid_url(cls, url) -> tuple[bool, Optional[dict]]: @@ -205,9 +207,9 @@ class VideoDownloader(Downloader, ABC): return final_filename, entry -channel_downloader = ChannelDownloader('video_channel') +channel_downloader = ChannelDownloader() # Videos may match the ChannelDownloader, give it a higher priority. -video_downloader = VideoDownloader('video', 40) +video_downloader = VideoDownloader(40) def get_or_create_channel(source_id: str = None, link: str = None, url: str = None, name: str = None) -> Channel: diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index d4ace302..de05d3c9 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -119,8 +119,8 @@ class TestVideosDownloaders(TestAPI): def setUp(self) -> None: super().setUp() - self.vd = VideoDownloader('video') - self.cd = ChannelDownloader('channel') + self.vd = VideoDownloader() + self.cd = ChannelDownloader() self.mgr = DownloadManager() self.mgr.register_downloader(self.vd) self.mgr.register_downloader(self.cd) diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 50916281..bfe9fd22 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -104,19 +104,20 @@ class Download(ModelHelper, Base): class Downloader: + name = None - def __init__(self, name: str, priority: int = 50): + def __init__(self, priority: int = 50, name: str = None): """ Lower `priority` means the downloader will be checked first. Valid priority: 0-100 Downloaders of equal priority will be used at random. """ - if not name: - raise ValueError('Downloader must have a name!') + if not name and not self.name: + raise NotImplementedError('Downloader must have a name!') if not 0 <= priority <= 100: raise ValueError(f'Priority of {priority} of {self} is invalid!') - self.name = name + self.name = self.name or name self.priority = priority self._kill = multiprocessing.Event() diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index bd49cb4b..88c00bf9 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -1,8 +1,11 @@ +from abc import ABC from datetime import datetime, timedelta from itertools import zip_longest from unittest import mock from unittest.mock import MagicMock +import pytest + from wrolpi import downloader from wrolpi.dates import local_timezone from wrolpi.db import get_db_context @@ -15,6 +18,7 @@ class PermissiveDownloader(Downloader): """ A testing Downloader which always says it's valid. """ + name = 'permissive' def valid_url(self, url: str): return True, None @@ -24,6 +28,7 @@ class HTTPDownloader(Downloader): """ A testing Downloader which says its valid when a URL starts with http/https """ + name = 'http' def valid_url(self, url: str): return url.startswith('https://') or url.startswith('http://'), None @@ -42,12 +47,12 @@ class TestDownloader(TestAPI): """ self.assertEqual(self.mgr.instances, tuple()) - http_downloader = HTTPDownloader('http') + http_downloader = HTTPDownloader() self.mgr.register_downloader(http_downloader) self.assertEqual(self.mgr.instances, (http_downloader,)) # PermissiveDownloader is first priority. - permissive_downloader = PermissiveDownloader('permissive', priority=0) + permissive_downloader = PermissiveDownloader(priority=0) self.mgr.register_downloader(permissive_downloader) self.assertEqual(self.mgr.instances, (permissive_downloader, http_downloader)) @@ -60,7 +65,7 @@ class TestDownloader(TestAPI): self.assertEqual(len(self.mgr.get_downloads(session)), 0) - http_downloader = HTTPDownloader('http') + http_downloader = HTTPDownloader() http_downloader.do_download.return_value = False self.mgr.register_downloader(http_downloader) @@ -76,11 +81,11 @@ class TestDownloader(TestAPI): def test_do_downloads(self): _, session = get_db_context() - http_downloader = HTTPDownloader('http') + http_downloader = HTTPDownloader() http_downloader.do_download.return_value = True self.mgr.register_downloader(http_downloader) - permissive_downloader = PermissiveDownloader('permissive', priority=100) + permissive_downloader = PermissiveDownloader(priority=100) permissive_downloader.do_download.return_value = False # returns a failure self.mgr.register_downloader(permissive_downloader) @@ -122,7 +127,7 @@ class TestDownloader(TestAPI): """ _, session = get_db_context() - http_downloader = HTTPDownloader('http') + http_downloader = HTTPDownloader() http_downloader.do_download.return_value = False self.mgr.register_downloader(http_downloader) @@ -148,7 +153,7 @@ class TestDownloader(TestAPI): """ _, session = get_db_context() - http_downloader = HTTPDownloader('http') + http_downloader = HTTPDownloader() http_downloader.do_download.return_value = True http_downloader.do_download: MagicMock # noqa self.mgr.register_downloader(http_downloader) @@ -224,7 +229,7 @@ class TestDownloader(TestAPI): """ Once-downloads over a month old should be deleted. """ - permissive_downloader = PermissiveDownloader('permissive', priority=0) + permissive_downloader = PermissiveDownloader(priority=0) self.mgr.register_downloader(permissive_downloader) _, session = get_db_context() @@ -267,7 +272,7 @@ class TestDownloader(TestAPI): """ Multiple downloads can be scheduled using DownloadManager.create_downloads. """ - http_downloader = HTTPDownloader('http', priority=0) + http_downloader = HTTPDownloader(priority=0) self.mgr.register_downloader(http_downloader) _, session = get_db_context() @@ -282,3 +287,17 @@ class TestDownloader(TestAPI): skip_download=True) downloads = self.mgr.get_downloads(session) self.assertEqual({i.url for i in downloads}, {'https://example.com/1', 'https://example.com/2'}) + + +def test_downloader_must_have_name(): + """ + A Downloader class must have a name. + """ + with pytest.raises(NotImplementedError): + Downloader() + + class D(Downloader, ABC): + pass + + with pytest.raises(NotImplementedError): + D()