diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 170151fe..7c9590c2 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -146,10 +146,10 @@ def request_archive(url: str): return singlefile, readability, screenshot -SINGLE_FILE_PATH = which('single-file', +SINGLE_FILE_BIN = which('single-file', '/usr/bin/single-file', # rpi ubuntu '/usr/local/bin/single-file', # debian - warn=True) + warn=True) CHROMIUM = which('chromium-browser', 'chromium', '/usr/bin/chromium-browser', # rpi ubuntu '/usr/bin/chromium', # debian @@ -159,10 +159,10 @@ CHROMIUM = which('chromium-browser', 'chromium', def local_singlefile(url: str): """Run the single-file executable to create an HTML file archive.""" - if not SINGLE_FILE_PATH.is_file(): + if not SINGLE_FILE_BIN.is_file(): raise FileNotFoundError(f'single-file not found') - cmd = (str(SINGLE_FILE_PATH), + cmd = (str(SINGLE_FILE_BIN), url, '--browser-executable-path', CHROMIUM, '--browser-args', '["--no-sandbox"]', @@ -193,19 +193,19 @@ def local_screenshot(url: str) -> bytes: return png -READABILITY_PATH = which('readability-extractor', +READABILITY_BIN = which('readability-extractor', '/usr/bin/readability-extractor', # rpi ubuntu '/usr/local/bin/readability-extractor', # debian - warn=True) + warn=True) def local_extract_readability(path: str, url: str) -> dict: """Extract the readability from an HTML file, typically from single-file.""" logger.info(f'readability for {url}') - if not READABILITY_PATH.is_file(): + if not READABILITY_BIN.is_file(): raise FileNotFoundError(f'Readability extractor not found') - cmd = (READABILITY_PATH, path, url) + cmd = (READABILITY_BIN, path, url) logger.debug(f'readability cmd: {cmd}') output = subprocess.check_output(cmd, timeout=60 * 3) output = json.loads(output) diff --git a/modules/files/lib.py b/modules/files/lib.py index 8df796d3..83fd55ae 100644 --- a/modules/files/lib.py +++ b/modules/files/lib.py @@ -91,12 +91,12 @@ def split_file_name(path: Path) -> List[str]: return words -FILE_PATH = which('file', '/usr/bin/file') +FILE_BIN = which('file', '/usr/bin/file') def get_mimetype(path: Path) -> str: """Get the mimetype using the builtin `file` command.""" - cmd = (FILE_PATH, '--mime-type', str(path.absolute())) + cmd = (FILE_BIN, '--mime-type', str(path.absolute())) output = subprocess.check_output(cmd) output = output.decode() mimetype = output.split(' ')[-1].strip() diff --git a/modules/map/lib.py b/modules/map/lib.py index 14811ab1..8ade3a80 100644 --- a/modules/map/lib.py +++ b/modules/map/lib.py @@ -125,7 +125,7 @@ async def import_files(files: List[str]): IMPORT_EVENT.clear() -BASH = which('bash', '/bin/bash', warn=True) +BASH_BIN = which('bash', '/bin/bash', warn=True) async def import_file(path: Path): @@ -143,7 +143,7 @@ async def import_file(path: Path): else: raise ValueError(f'Cannot import unknown file! {path}') - cmd = f'{BASH} {PROJECT_DIR}/scripts/import_map.sh {path.absolute()}' + cmd = f'{BASH_BIN} {PROJECT_DIR}/scripts/import_map.sh {path.absolute()}' logger.debug(f'Running import script: {cmd}') start = now() proc = await asyncio.create_subprocess_shell(cmd, stderr=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE) diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index 03c66426..173c583e 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -9,7 +9,7 @@ from typing import Tuple, List, Optional import yt_dlp.utils from sqlalchemy.orm import Session from yt_dlp import YoutubeDL -from yt_dlp.extractor import YoutubeTabIE +from yt_dlp.extractor import YoutubeTabIE # noqa from yt_dlp.utils import UnsupportedError, DownloadError from wrolpi.cmd import which @@ -17,7 +17,7 @@ from wrolpi.common import logger, extract_domain, get_media_directory from wrolpi.dates import now from wrolpi.db import get_db_session, get_db_curs from wrolpi.db import optional_session -from wrolpi.downloader import Downloader, Download, DOWNLOAD_MANAGER_CONFIG, DownloadResult +from wrolpi.downloader import Downloader, Download, DownloadResult from wrolpi.errors import UnknownChannel, ChannelURLEmpty, UnrecoverableDownloadError from wrolpi.vars import PYTEST from .channel.lib import create_channel, get_channel @@ -84,13 +84,18 @@ class ChannelDownloader(Downloader, ABC): def do_download(self, download: Download) -> DownloadResult: """Update a Channel's catalog, then schedule downloads of every missing video.""" info = extract_info(download.url, process=False) + # Resolve the entries generator. + info['entries'] = list(info['entries']) + download.info_json = info + download.sub_downloader = video_downloader.name + if session := Session.object_session(download): + # May not have a session during testing. + session.commit() name = info['uploader'] channel_source_id = info['channel_id'] channel = get_or_create_channel(channel_source_id, download.url, name) channel.dict() # get all attributes while we have the session. - # Resolve the entries generator. - info['entries'] = list(info['entries']) if self.is_a_playlist(info): result = self.upsert_playlist_videos(download, channel) @@ -126,32 +131,10 @@ class ChannelDownloader(Downloader, ABC): def upsert_playlist_videos(download: Download, channel: Channel) -> DownloadResult: """Get a list of all videos in a playlist, schedule downloads for any missing videos.""" try: - # Playlists require more information to download. - info = extract_info(download.url, process=True) - # Resolve the entries generator. - info['entries'] = list(info['entries']) - - # Get all Video.source_id of this Channel. - with get_db_session() as session: - existing_ids = [i for (i,) in session.query(Video.source_id).filter(Video.channel_id == channel.id)] - - # 'id' is our 'source_id' - if any(i['id'] not in existing_ids for i in info['entries']): - # At least one video in the playlist is not in the Channel catalog, update the catalog. - channel_info = extract_info(channel.url, process=True) - update_channel_catalog(channel, channel_info) - - with get_db_session() as session: - channel_video_urls = [i for (i,) in session.query(Video.url).filter(Video.channel_id == channel.id, - Video.url != None)] - - skip_urls = set(DOWNLOAD_MANAGER_CONFIG.skip_urls) - # All videos in the channel catalog that have not been downloaded (most will not be in the playlist). - missing_source_ids = {source_id for _, source_id, _ in _find_all_missing_videos(channel.id)} - downloads = [i['webpage_url'] for i in info['entries'] if - i['id'] in missing_source_ids and i['webpage_url'] not in skip_urls] - # A video may be in a playlist, but not published in the channel catalog. - downloads.extend(i['webpage_url'] for i in info['entries'] if i['webpage_url'] not in channel_video_urls) + # Get all videos in the playlist + downloads = [i['url'] for i in download.info_json['entries']] + # Only download those that have not yet been downloaded. + downloads = [i for i in downloads if not video_downloader.already_downloaded(i)] return DownloadResult(success=True, location=f'/videos/channel/{channel.id}/video', downloads=downloads) except Exception: logger.warning(f'Failed to update catalog of playlist {download.url}') @@ -179,7 +162,11 @@ class VideoDownloader(Downloader, ABC): @optional_session def already_downloaded(self, url: str, session: Session = None) -> Optional[Video]: - video = session.query(Video).filter_by(url=url).one_or_none() + # We only consider a video record with a video file as "downloaded". + video = session.query(Video).filter( + Video.url == url, + Video.video_path != None, + ).one_or_none() return video @classmethod @@ -244,6 +231,7 @@ class VideoDownloader(Downloader, ABC): '--write-info-json', '--merge-output-format', PREFERRED_VIDEO_EXTENSION, '-o', file_name_format, + '--no-cache-dir', url, ) return_code, logs = self.process_runner(url, cmd, out_dir) diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index 9ef3e56e..7950f821 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -408,7 +408,7 @@ example_playlist_json = { 'ie_key': 'Youtube', 'title': 'video 2 title', 'uploader': None, - 'url': 'video_2_url', + 'url': 'https://youtube.com/watch?v=video_2_url', 'view_count': 1413, 'webpage_url': 'https://youtube.com/watch?v=video_2_url'}, {'_type': 'url', @@ -418,7 +418,7 @@ example_playlist_json = { 'ie_key': 'Youtube', 'title': 'video 1 title', 'uploader': None, - 'url': 'video_1_url', + 'url': 'https://youtube.com/watch?v=video_1_url', 'view_count': 58504, 'webpage_url': 'https://youtube.com/watch?v=video_1_url'}, {'_type': 'url', @@ -428,7 +428,7 @@ example_playlist_json = { 'ie_key': 'Youtube', 'title': 'video 3 title', 'uploader': None, - 'url': 'video_3_url', + 'url': 'https://youtube.com/watch?v=video_3_url', 'view_count': 58504, 'webpage_url': 'https://youtube.com/watch?v=video_3_url'}, ], @@ -463,14 +463,7 @@ def test_download_playlist(test_session, test_directory, mock_video_extract_info poster_path='poster exists')) test_session.commit() - mock_video_extract_info.side_effect = [ - example_playlist_json, # Playlist info is fetched first. - example_playlist_json, # Playlist info is fetched again for more details. - example_channel_json, # Video 3 causes channel catalog to be fetched. - example_video_json, - example_video_json, - example_video_json, - ] + mock_video_extract_info.return_value = example_playlist_json # Playlist info is fetched first. with mock.patch('modules.videos.downloader.VideoDownloader.do_download') as mock_video_do_download: mock_video_do_download.return_value = DownloadResult(success=True) # Don't download the videos. diff --git a/wrolpi/admin.py b/wrolpi/admin.py index 2f4812c1..760e98df 100644 --- a/wrolpi/admin.py +++ b/wrolpi/admin.py @@ -7,10 +7,10 @@ from wrolpi.vars import DEFAULT_CPU_FREQUENCY, DOCKERIZED logger = logger.getChild(__name__) -SUDO = which('sudo', '/usr/bin/sudo') -NMCLI = which('nmcli', '/usr/bin/nmcli') -CPUFREQ_INFO = which('cpufreq-info', '/usr/bin/cpufreq-info') -CPUFREQ_SET = which('cpufreq-set', '/usr/bin/cpufreq-set') +SUDO_BIN = which('sudo', '/usr/bin/sudo') +NMCLI_BIN = which('nmcli', '/usr/bin/nmcli') +CPUFREQ_INFO_BIN = which('cpufreq-info', '/usr/bin/cpufreq-info') +CPUFREQ_SET_BIN = which('cpufreq-set', '/usr/bin/cpufreq-set') POWER_SAVE_FREQ = 'powersave' # noqa @@ -23,10 +23,10 @@ class HotspotStatus(enum.Enum): def hotspot_status() -> HotspotStatus: - if NMCLI is None: + if NMCLI_BIN is None: return HotspotStatus.unknown - cmd = (NMCLI,) + cmd = (NMCLI_BIN,) try: output = subprocess.check_output(cmd).decode().strip() except FileNotFoundError as e: @@ -60,14 +60,14 @@ def enable_hotspot(): return enable_hotspot() elif status == HotspotStatus.disconnected: # Radio is on, but not connected. Good, turn it into a hotspot. - cmd = (SUDO, NMCLI, 'device', 'wifi', 'hotspot', 'ifname', WROLPI_CONFIG.hotspot_device, + cmd = (SUDO_BIN, NMCLI_BIN, 'device', 'wifi', 'hotspot', 'ifname', WROLPI_CONFIG.hotspot_device, 'ssid', WROLPI_CONFIG.hotspot_ssid, 'password', WROLPI_CONFIG.hotspot_password) subprocess.check_call(cmd) logger.info('Enabling hotspot successful') return True elif status == HotspotStatus.unavailable: # Radio is not on, turn it on. - cmd = (SUDO, NMCLI, 'radio', 'wifi', 'on') + cmd = (SUDO_BIN, NMCLI_BIN, 'radio', 'wifi', 'on') subprocess.check_call(cmd) logger.info('Turned on hotspot radio.') return enable_hotspot() @@ -77,7 +77,7 @@ def enable_hotspot(): def disable_hotspot(): """Turn off the wifi interface.""" - cmd = (SUDO, NMCLI, 'radio', 'wifi', 'off') + cmd = (SUDO_BIN, NMCLI_BIN, 'radio', 'wifi', 'off') try: subprocess.check_call(cmd) logger.info('Disabling hotspot successful') @@ -105,10 +105,10 @@ GOVERNOR_MAP = { def throttle_status() -> GovernorStatus: - if not CPUFREQ_INFO: + if not CPUFREQ_INFO_BIN: return GovernorStatus.unknown - cmd = (CPUFREQ_INFO,) + cmd = (CPUFREQ_INFO_BIN,) try: output = subprocess.check_output(cmd) except FileNotFoundError as e: @@ -128,7 +128,7 @@ def throttle_status() -> GovernorStatus: def throttle_cpu_on() -> bool: """Call cpufreq-set to throttle CPU.""" try: - cmd = (SUDO, CPUFREQ_SET, '-g', POWER_SAVE_FREQ) + cmd = (SUDO_BIN, CPUFREQ_SET_BIN, '-g', POWER_SAVE_FREQ) subprocess.check_call(cmd) return True except FileNotFoundError: @@ -139,7 +139,7 @@ def throttle_cpu_on() -> bool: def throttle_cpu_off() -> bool: """Call cpufreq-set to un-throttle CPU.""" try: - cmd = (SUDO, CPUFREQ_SET, '-g', DEFAULT_CPU_FREQUENCY) + cmd = (SUDO_BIN, CPUFREQ_SET_BIN, '-g', DEFAULT_CPU_FREQUENCY) subprocess.check_call(cmd) return True except FileNotFoundError: diff --git a/wrolpi/test/test_admin.py b/wrolpi/test/test_admin.py index 41ebb526..f046837e 100644 --- a/wrolpi/test/test_admin.py +++ b/wrolpi/test/test_admin.py @@ -23,7 +23,7 @@ def test_enable_hotspot_disconnected(): """Enable hotspot with wifi on.""" with mock.patch('wrolpi.admin.hotspot_status') as mock_hotspot_status, \ mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.NMCLI', PosixPath('/usr/bin/nmcli')): + mock.patch('wrolpi.admin.NMCLI_BIN', PosixPath('/usr/bin/nmcli')): mock_hotspot_status.return_value = HotspotStatus.disconnected mock_subprocess.check_output.return_value = 0 assert admin.enable_hotspot() is True @@ -36,7 +36,7 @@ def test_enable_hotspot_unavailable(): """Enable hotspot with wifi off.""" with mock.patch('wrolpi.admin.hotspot_status') as mock_hotspot_status, \ mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.NMCLI', PosixPath('/usr/bin/nmcli')): + mock.patch('wrolpi.admin.NMCLI_BIN', PosixPath('/usr/bin/nmcli')): mock_hotspot_status.side_effect = [HotspotStatus.unavailable, HotspotStatus.disconnected] mock_subprocess.check_output.return_value = 0 assert admin.enable_hotspot() is True @@ -52,7 +52,7 @@ def test_enable_hotspot_connected(): """Enable hotspot with the hotspot alaready enabled.""" with mock.patch('wrolpi.admin.hotspot_status') as mock_hotspot_status, \ mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.NMCLI', PosixPath('/usr/bin/nmcli')): + mock.patch('wrolpi.admin.NMCLI_BIN', PosixPath('/usr/bin/nmcli')): mock_hotspot_status.side_effect = [ HotspotStatus.connected, HotspotStatus.unavailable, @@ -73,7 +73,7 @@ def test_change_hotspot(test_config): """The hotspot can be configured.""" with mock.patch('wrolpi.admin.hotspot_status') as mock_hotspot_status, \ mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.NMCLI', PosixPath('/usr/bin/nmcli')): + mock.patch('wrolpi.admin.NMCLI_BIN', PosixPath('/usr/bin/nmcli')): mock_subprocess.check_output.return_value = 0 mock_hotspot_status.return_value = HotspotStatus.disconnected assert admin.enable_hotspot() is True @@ -85,7 +85,7 @@ def test_change_hotspot(test_config): WROLPI_CONFIG.hotspot_password = '$uper Secure *#' with mock.patch('wrolpi.admin.hotspot_status') as mock_hotspot_status, \ mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.NMCLI', PosixPath('/usr/bin/nmcli')): + mock.patch('wrolpi.admin.NMCLI_BIN', PosixPath('/usr/bin/nmcli')): mock_subprocess.check_output.return_value = 0 mock_hotspot_status.return_value = HotspotStatus.disconnected assert admin.enable_hotspot() is True @@ -96,7 +96,7 @@ def test_change_hotspot(test_config): def test_throttle_on(): with mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.CPUFREQ_SET', PosixPath('/usr/bin/cpufreq-set')): + mock.patch('wrolpi.admin.CPUFREQ_SET_BIN', PosixPath('/usr/bin/cpufreq-set')): assert admin.throttle_cpu_on() mock_subprocess.check_call.assert_called_once_with( (PosixPath('/usr/bin/sudo'), PosixPath('/usr/bin/cpufreq-set'), '-g', 'powersave') @@ -105,7 +105,7 @@ def test_throttle_on(): def test_throttle_off(): with mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.CPUFREQ_SET', PosixPath('/usr/bin/cpufreq-set')): + mock.patch('wrolpi.admin.CPUFREQ_SET_BIN', PosixPath('/usr/bin/cpufreq-set')): assert admin.throttle_cpu_off() mock_subprocess.check_call.assert_called_once_with( (PosixPath('/usr/bin/sudo'), PosixPath('/usr/bin/cpufreq-set'), '-g', 'ondemand') diff --git a/wrolpi/test/test_root_api.py b/wrolpi/test/test_root_api.py index bd678f47..7f3964e9 100644 --- a/wrolpi/test/test_root_api.py +++ b/wrolpi/test/test_root_api.py @@ -62,8 +62,8 @@ class TestRootAPI(TestAPI): assert json.loads(response.body) == {'valid': False, 'regex': '.*(missing parenthesis.*'} def test_get_settings(self): - with mock.patch('wrolpi.admin.CPUFREQ_INFO', '/usr/bin/cpufreq-info'), \ - mock.patch('wrolpi.admin.NMCLI', '/usr/bin/nmcli'): + with mock.patch('wrolpi.admin.CPUFREQ_INFO_BIN', '/usr/bin/cpufreq-info'), \ + mock.patch('wrolpi.admin.NMCLI_BIN', '/usr/bin/nmcli'): request, response = api_app.test_client.get('/api/settings') self.assertOK(response) @@ -216,7 +216,7 @@ def test_hotspot_settings(test_session, test_client, test_config): @skip_circleci def test_throttle_toggle(test_session, test_client, test_config): with mock.patch('wrolpi.admin.subprocess') as mock_subprocess, \ - mock.patch('wrolpi.admin.CPUFREQ_INFO', "this value isn't even used"): + mock.patch('wrolpi.admin.CPUFREQ_INFO_BIN', "this value isn't even used"): mock_subprocess.check_output.side_effect = [ b'wlan0: unavailable', b'The governor "ondemand" may decide ',