diff --git a/app/src/components/Admin.js b/app/src/components/Admin.js
index 9898582e..19ff55c8 100644
--- a/app/src/components/Admin.js
+++ b/app/src/components/Admin.js
@@ -26,6 +26,7 @@ import {
import TimezoneSelect from 'react-timezone-select';
import {
DisableDownloadsToggle,
+ HelpPopup,
HotspotToggle,
PageContainer,
secondsToDate,
@@ -51,6 +52,7 @@ class Settings extends React.Component {
ready: false,
download_on_startup: null,
+ download_timeout: null,
hotspot_on_startup: null,
hotspot_password: null,
hotspot_ssid: null,
@@ -71,6 +73,7 @@ class Settings extends React.Component {
ready: true,
disabled: settings.wrol_mode,
download_on_startup: settings.download_on_startup,
+ download_timeout: settings.download_timeout || '',
hotspot_on_startup: settings.hotspot_on_startup,
hotspot_password: settings.hotspot_password,
hotspot_ssid: settings.hotspot_ssid,
@@ -89,6 +92,7 @@ class Settings extends React.Component {
this.setState({disabled: true, pending: true});
let settings = {
download_on_startup: this.state.download_on_startup,
+ download_timeout: this.state.download_timeout ? parseInt(this.state.download_timeout) : 0,
hotspot_on_startup: this.state.hotspot_on_startup,
hotspot_password: this.state.hotspot_password,
hotspot_ssid: this.state.hotspot_ssid,
@@ -113,6 +117,15 @@ class Settings extends React.Component {
this.setState({qrCodeValue});
}
+ handleTimeoutChange = async (e, name, value) => {
+ if (e) {
+ e.preventDefault()
+ }
+ // Restrict timeout to numbers.
+ value = value.replace(/[^\d]/,'');
+ this.setState({[name]: value});
+ }
+
render() {
if (this.state.ready === false) {
return
@@ -121,6 +134,7 @@ class Settings extends React.Component {
let {
disabled,
download_on_startup,
+ download_timeout,
hotspot_on_startup,
hotspot_password,
hotspot_ssid,
@@ -174,6 +188,19 @@ class Settings extends React.Component {
onChange={(e, d) => this.handleInputChange(e, 'throttle_on_startup', d.checked)}
/>
+
+
+ Download Timeout
+
+ >}
+ value={download_timeout}
+ disabled={disabled || download_timeout === null}
+ onChange={(e, d) => this.handleTimeoutChange(e, 'download_timeout', d.value)}
+ />
+
+
int:
+ return self._config['download_timeout']
+
+ @download_timeout.setter
+ def download_timeout(self, value: int):
+ self.update({'download_timeout': value})
+
@property
def hotspot_on_startup(self) -> bool:
return self._config['hotspot_on_startup']
diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py
index 85a5fa74..a5a05254 100644
--- a/wrolpi/downloader.py
+++ b/wrolpi/downloader.py
@@ -18,7 +18,7 @@ from sqlalchemy import Column, Integer, String, Text
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.orm import Session
-from wrolpi.common import Base, ModelHelper, logger, wrol_mode_check, zig_zag, ConfigFile
+from wrolpi.common import Base, ModelHelper, logger, wrol_mode_check, zig_zag, ConfigFile, WROLPI_CONFIG
from wrolpi.dates import TZDateTime, now, Seconds, local_timezone, recursive_replace_tz
from wrolpi.db import get_db_session, get_db_curs, get_db_context, optional_session
from wrolpi.errors import InvalidDownload, UnrecoverableDownloadError
@@ -128,11 +128,12 @@ class Download(ModelHelper, Base):
class Downloader:
- name = None
- pretty_name = None
- listable = True
+ name: str = None
+ pretty_name: str = None
+ listable: bool = True
+ timeout: int = None
- def __init__(self, priority: int = 50, name: str = None):
+ def __init__(self, priority: int = 50, name: str = None, timeout: int = None):
"""
Lower `priority` means the downloader will be checked first. Valid priority: 0-100
@@ -143,8 +144,9 @@ class Downloader:
if not 0 <= priority <= 100:
raise ValueError(f'Priority of {priority} of {self} is invalid!')
- self.name = self.name or name
- self.priority = priority
+ self.name: str = self.name or name
+ self.priority: int = priority
+ self.timeout: int = timeout or self.timeout
self._kill = multiprocessing.Event()
self._manager: DownloadManager = None # noqa
@@ -176,28 +178,35 @@ class Downloader:
self._manager = value
def kill(self):
- """
- Kill the running download for this Downloader.
- """
+ """Kill the running download for this Downloader."""
if not self._kill.is_set():
self._kill.set()
def clear(self):
- """
- Clear any "kill" request for this Downloader.
- """
+ """Clear any "kill" request for this Downloader."""
if self._kill.is_set():
self._kill.clear()
- def process_runner(self, url: str, cmd: List[str], cwd: pathlib.Path, **kwargs) -> Tuple[int, dict]:
+ def process_runner(self, url: str, cmd: Tuple[str, ...], cwd: pathlib.Path, timeout: int = None,
+ **kwargs) -> Tuple[int, dict]:
"""
Run a subprocess using the provided arguments. This process can be killed by the Download Manager.
+
+ Global timeout takes precedence over the timeout argument, unless it is 0. (Smaller global timeout wins)
"""
logger.debug(f'{self} launching download process with args: {" ".join(cmd)}')
+ start = now()
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, **kwargs)
pid = proc.pid
logger.debug(f'{self} launched download process {pid=} for {url}')
+ # Timeout can be any positive integer. Global download timeout takes precedence, unless it is 0.
+ # A timeout of 0 means the download will never be killed.
+ if timeout is None:
+ timeout = self.timeout
+ if WROLPI_CONFIG.download_timeout and timeout > WROLPI_CONFIG.download_timeout:
+ timeout = WROLPI_CONFIG.download_timeout
+
try:
while True:
try:
@@ -207,13 +216,17 @@ class Downloader:
except subprocess.TimeoutExpired:
pass
+ if timeout and (elapsed := now() - start).total_seconds() > timeout:
+ logger.warning(f'Download has exceeded its timeout {elapsed=}')
+ self.kill()
+
if self._kill.is_set():
logger.warning(f'Killing download {pid=}')
proc.kill()
proc.poll()
- raise ChildProcessError(f'Download {pid=} was killed!')
+ break
finally:
- self._kill.clear()
+ self.clear()
# Output all logs from the process.
# TODO is there a way to stream this output while the process is running?
diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py
index f9d58fd0..37f7ffd0 100644
--- a/wrolpi/root_api.py
+++ b/wrolpi/root_api.py
@@ -120,6 +120,7 @@ def get_settings(_: Request):
settings = {
'download_on_startup': config.download_on_startup,
+ 'download_timeout': config.download_timeout,
'hotspot_on_startup': config.hotspot_on_startup,
'hotspot_password': config.hotspot_password,
'hotspot_ssid': config.hotspot_ssid,
diff --git a/wrolpi/schema.py b/wrolpi/schema.py
index 120ceb66..8909b201 100644
--- a/wrolpi/schema.py
+++ b/wrolpi/schema.py
@@ -15,6 +15,7 @@ class SettingsResponse:
@dataclass
class SettingsRequest:
download_on_startup: Optional[bool] = None
+ download_timeout: Optional[int] = None
hotspot_on_startup: Optional[bool] = None
hotspot_password: Optional[str] = None
hotspot_ssid: Optional[str] = None
diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py
index 93ee46af..3aa9b4a3 100644
--- a/wrolpi/test/test_downloader.py
+++ b/wrolpi/test/test_downloader.py
@@ -200,7 +200,7 @@ async def test_download_wrol_mode(test_session, test_download_manager):
await test_download_manager.do_downloads_sync()
-def test_download_get_downloader(test_session, test_download_manager,):
+def test_download_get_downloader(test_session, test_download_manager, ):
"""
A Download can find it's Downloader.
"""
@@ -364,3 +364,37 @@ def test_skip_urls(test_session, test_download_manager):
], downloader_name=HTTPDownloader.name)
downloads = test_download_manager.get_downloads(test_session)
assert {i.url for i in downloads} == {'https://example.com/1', 'https://example.com/2'}
+
+
+def test_process_runner_timeout(test_directory):
+ """A Downloader can cancel it's download using a timeout."""
+ # Default timeout of 3 seconds.
+ downloader = Downloader(0, 'downloader', timeout=3)
+
+ # Default timeout is obeyed.
+ start = datetime.now()
+ downloader.process_runner('https://example.com', ('sleep', '8'), test_directory)
+ elapsed = datetime.now() - start
+ assert 3 < elapsed.total_seconds() < 4
+
+ # One-off timeout is obeyed.
+ start = datetime.now()
+ downloader.process_runner('https://example.com', ('sleep', '8'), test_directory, timeout=1)
+ elapsed = datetime.now() - start
+ assert 1 < elapsed.total_seconds() < 2
+
+ # Zero timeout means no timeout.
+ start = datetime.now()
+ downloader.process_runner('https://example.com', ('sleep', '8'), test_directory, timeout=0)
+ elapsed = datetime.now() - start
+ assert 7 < elapsed.total_seconds() < 10
+
+ # This Downloader's timeout is ignored in favor of the global timeout.
+ from wrolpi.common import WROLPI_CONFIG
+ WROLPI_CONFIG.download_timeout = 3
+
+ # Global timeout is obeyed.
+ start = datetime.now()
+ downloader.process_runner('https://example.com', ('sleep', '8'), test_directory)
+ elapsed = datetime.now() - start
+ assert 2 < elapsed.total_seconds() < 4
diff --git a/wrolpi/version.py b/wrolpi/version.py
index 5b450d27..7016889c 100644
--- a/wrolpi/version.py
+++ b/wrolpi/version.py
@@ -1,4 +1,4 @@
-__version__ = '0.5.0-beta'
+__version__ = '0.5.1-beta'
import subprocess