From f5e6dbde7a1749416d17be8789029feffc7bfce1 Mon Sep 17 00:00:00 2001 From: Roland Date: Wed, 5 Apr 2023 10:40:16 -0600 Subject: [PATCH] Switching all download manager config uses to use getter function for testing. --- wrolpi/conftest.py | 13 +++++------ wrolpi/downloader.py | 41 ++++++++++++++++++++++------------ wrolpi/test/test_downloader.py | 13 +++++------ wrolpi/test/test_root_api.py | 6 ++--- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/wrolpi/conftest.py b/wrolpi/conftest.py index 9502ef4c..b94ac0e3 100644 --- a/wrolpi/conftest.py +++ b/wrolpi/conftest.py @@ -19,7 +19,6 @@ from uuid import uuid1, uuid4 import pytest import sqlalchemy -import yaml from PIL import Image from sanic_testing.testing import SanicTestClient from sqlalchemy.engine import Engine, create_engine @@ -30,7 +29,8 @@ from wrolpi.common import iterify, log_level_context from wrolpi.common import set_test_media_directory, Base, set_test_config from wrolpi.dates import set_test_now from wrolpi.db import postgres_engine, get_db_args -from wrolpi.downloader import DownloadManager, DownloadResult, set_test_download_manager_config, Download, Downloader +from wrolpi.downloader import DownloadManager, DownloadResult, Download, Downloader, \ + downloads_manager_config_context from wrolpi.files import lib as files_lib from wrolpi.root_api import BLUEPRINTS, api_app from wrolpi.tags import Tag @@ -135,11 +135,10 @@ def test_client() -> SanicTestClient: @pytest.fixture def test_download_manager_config(test_directory): - (test_directory / 'config').mkdir(exist_ok=True) - config_path = test_directory / 'config/download_manager.yaml' - set_test_download_manager_config(True) - yield config_path - set_test_download_manager_config(False) + with downloads_manager_config_context(): + (test_directory / 'config').mkdir(exist_ok=True) + config_path = test_directory / 'config/download_manager.yaml' + yield config_path @pytest.fixture diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index f010c785..8dd0acc7 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import logging import multiprocessing import os @@ -507,7 +508,7 @@ class DownloadManager: download = self.get_download(session, url=url) if not download: - if url in DOWNLOAD_MANAGER_CONFIG.skip_urls: + if url in get_download_manager_config().skip_urls: raise InvalidDownload( f'Refusing to download {url} because it is in the download_manager.yaml skip list') download = Download(url=url, status='new') @@ -530,10 +531,10 @@ class DownloadManager: with session.transaction: for url in urls: - if url in DOWNLOAD_MANAGER_CONFIG.skip_urls and reset_attempts: + if url in get_download_manager_config().skip_urls and reset_attempts: # User manually entered this download, remove it from the skip list. self.remove_from_skip_list(url) - elif url in DOWNLOAD_MANAGER_CONFIG.skip_urls: + elif url in get_download_manager_config().skip_urls: self.log_warning(f'Skipping {url} because it is in the download_manager.yaml skip list.') continue @@ -990,13 +991,13 @@ class DownloadManager: @staticmethod def add_to_skip_list(*urls: str): - DOWNLOAD_MANAGER_CONFIG.skip_urls = list(set(DOWNLOAD_MANAGER_CONFIG.skip_urls) | set(urls)) - DOWNLOAD_MANAGER_CONFIG.save() + get_download_manager_config().skip_urls = list(set(get_download_manager_config().skip_urls) | set(urls)) + get_download_manager_config().save() @staticmethod def remove_from_skip_list(url: str): - DOWNLOAD_MANAGER_CONFIG.skip_urls = [i for i in DOWNLOAD_MANAGER_CONFIG.skip_urls if i != url] - DOWNLOAD_MANAGER_CONFIG.save() + get_download_manager_config().skip_urls = [i for i in get_download_manager_config().skip_urls if i != url] + get_download_manager_config().save() # The global DownloadManager. This should be used everywhere! @@ -1028,13 +1029,25 @@ class DownloadMangerConfig(ConfigFile): DOWNLOAD_MANAGER_CONFIG: DownloadMangerConfig = DownloadMangerConfig() +TEST_DOWNLOAD_MANAGER_CONFIG: DownloadMangerConfig = None -def set_test_download_manager_config(enabled: bool): +@contextlib.contextmanager +def downloads_manager_config_context(): + """Used to create a test config.""" + global TEST_DOWNLOAD_MANAGER_CONFIG + TEST_DOWNLOAD_MANAGER_CONFIG = DownloadMangerConfig() + yield + TEST_DOWNLOAD_MANAGER_CONFIG = None + + +def get_download_manager_config() -> DownloadMangerConfig: + global TEST_DOWNLOAD_MANAGER_CONFIG + if isinstance(TEST_DOWNLOAD_MANAGER_CONFIG, ConfigFile): + return TEST_DOWNLOAD_MANAGER_CONFIG + global DOWNLOAD_MANAGER_CONFIG - DOWNLOAD_MANAGER_CONFIG = None - if enabled: - DOWNLOAD_MANAGER_CONFIG = DownloadMangerConfig() + return DOWNLOAD_MANAGER_CONFIG @optional_session(commit=False) @@ -1055,9 +1068,9 @@ async def save_downloads_config(session: Session): sub_downloader=download.sub_downloader, url=download.url, )) - if config != DOWNLOAD_MANAGER_CONFIG.downloads: + if config != get_download_manager_config().downloads: # Only save if there are changes. - DOWNLOAD_MANAGER_CONFIG.downloads = config + get_download_manager_config().downloads = config @optional_session @@ -1073,7 +1086,7 @@ async def import_downloads_config(session: Session): try: logger.warning('Importing downloads in config') - downloads_by_url = {i['url']: i for i in DOWNLOAD_MANAGER_CONFIG.downloads} + downloads_by_url = {i['url']: i for i in get_download_manager_config().downloads} existing_downloads = list(session.query(Download)) for existing in existing_downloads: download = downloads_by_url.pop(existing.url, None) diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index 2af04b9b..e1836585 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -13,7 +13,7 @@ import pytz from wrolpi.dates import Seconds from wrolpi.db import get_db_context from wrolpi.downloader import Downloader, Download, DownloadFrequency, DownloadResult, import_downloads_config, \ - RSSDownloader + RSSDownloader, get_download_manager_config from wrolpi.errors import UnrecoverableDownloadError, InvalidDownload, WROLModeEnabled from wrolpi.test.common import assert_dict_contains @@ -250,8 +250,7 @@ async def test_max_attempts(test_session, test_download_manager, test_downloader async def test_skip_urls(test_session, test_download_manager, test_client, assert_download_urls, test_downloader): """The DownloadManager will not create downloads for URLs in it's skip list.""" _, session = get_db_context() - from wrolpi.downloader import DOWNLOAD_MANAGER_CONFIG - DOWNLOAD_MANAGER_CONFIG.skip_urls = ['https://example.com/skipme'] + get_download_manager_config().skip_urls = ['https://example.com/skipme'] test_downloader.do_download = MagicMock() test_downloader.do_download.return_value = DownloadResult(success=True) @@ -263,14 +262,14 @@ async def test_skip_urls(test_session, test_download_manager, test_client, asser ], downloader_name=test_downloader.name) await test_download_manager.wait_for_all_downloads() assert_download_urls({'https://example.com/1', 'https://example.com/2'}) - assert DOWNLOAD_MANAGER_CONFIG.skip_urls == ['https://example.com/skipme'] + assert get_download_manager_config().skip_urls == ['https://example.com/skipme'] test_download_manager.delete_completed() # The user can start a download even if the URL is in the skip list. test_download_manager.create_download('https://example.com/skipme', test_downloader.name, reset_attempts=True) assert_download_urls({'https://example.com/skipme'}) - assert DOWNLOAD_MANAGER_CONFIG.skip_urls == [] + assert get_download_manager_config().skip_urls == [] @pytest.mark.asyncio @@ -363,7 +362,7 @@ async def test_downloads_config(test_session, test_client, test_download_manager await asyncio.sleep(0) # Downloads were saved to config. - assert DOWNLOAD_MANAGER_CONFIG.downloads, 'No downloads were saved' + assert get_download_manager_config().downloads, 'No downloads were saved' expected = [ # https://example.com/1 is missing because it is completed. dict( @@ -380,7 +379,7 @@ async def test_downloads_config(test_session, test_client, test_download_manager sub_downloader=None, ), ] - for download, expected in zip_longest(DOWNLOAD_MANAGER_CONFIG.downloads, expected): + for download, expected in zip_longest(get_download_manager_config().downloads, expected): assert_dict_contains(download, expected) # Delete a Downloads, so we can import what was saved. diff --git a/wrolpi/test/test_root_api.py b/wrolpi/test/test_root_api.py index b2b4d588..4552061f 100644 --- a/wrolpi/test/test_root_api.py +++ b/wrolpi/test/test_root_api.py @@ -8,7 +8,7 @@ from wrolpi.admin import HotspotStatus from wrolpi.common import get_config from wrolpi.dates import strptime from wrolpi.db import get_db_session -from wrolpi.downloader import Download +from wrolpi.downloader import Download, get_download_manager_config from wrolpi.root_api import api_app from wrolpi.test.common import TestAPI, wrap_test_db, skip_circleci @@ -232,8 +232,6 @@ def test_throttle_toggle(test_session, test_client, test_config): def test_clear_downloads(test_session, test_client, test_config, test_download_manager_config): - from wrolpi.downloader import DOWNLOAD_MANAGER_CONFIG - with get_db_session(commit=True) as session: d1 = Download(url='https://example.com/1', status='complete') d2 = Download(url='https://example.com/2', status='pending') @@ -298,7 +296,7 @@ def test_clear_downloads(test_session, test_client, test_config, test_download_m check_downloads(response, once_downloads, recurring_downloads, status_code=HTTPStatus.OK) # Failed once-downloads will not be downloaded again. - assert DOWNLOAD_MANAGER_CONFIG.skip_urls == ['https://example.com/5', ] + assert get_download_manager_config().skip_urls == ['https://example.com/5', ] def test_get_status(test_client, test_session):